Bug in KAction::initPrivate

[Andreas Beckermann]

On Thursday 26 September 2002 09:37, Ellis Whitehead wrote:
> Hi Simon,
>
> On Wednesday 25 September 2002 00:47, Simon Hausmann wrote:
> > ...
> > IMHO this is a case of a too-magic API. The only similar case that I
> > can think of is QSignal, which does a similar matching for an
> > integer signature in the slot, but it is not bound to another
> > completely unrelated property, like KAction is with the object name.
> >
> > (ah, I just see that this is apparently a change that is new to KDE
> > 3.1, so we could still revert it if we find a better solution, or?)

Well - *if* you do it *please* do it asap. I'll have to change boson's code, 
too or I'll have to tell users to use KDE >= 3.0.1 but <= 3.1 for boson 0.7. 

> > Looking at the code I actually don't see a reason for it. I might be
> > missing something (please correct me if wrong), but isn't the
> > functionality that KAction tries to do in magic (and apparently
> > fails under certain conditions) exactly what QSignalMapper solves as
> > separate class, even more flexible (allows mapping using a string in
> > addition to the int mapping) and well-documented?
> >
> > I mean, just from reading code it's more obvious to me what
> >
> > QSignalMapper *desktopNumberMapper = new QSignalMapper( this );
> >
> > connect( desktopNumberMapper, SIGNAL( mapped( int ) ),
> >          this, SLOT( moveToDesktop( int ) ) );
> >
> > moveToDesktopAction = new KAction( .., desktopNumberMapper, SLOT( map()
> > ), actionCollection() );
> >
> > desktopNumberMapper->setMapping( moveToDesktopAction, 1 );
> >
> > does, than
> >
> > foob = new KAction( ..., this, SLOT( sendToDesktop( int ),
> >                     actionCollection(), "sendToDesktop{1}" )

Yes you are right - once you have found this solution this is more obvious. 
But I for example was aware of QSignalMapper and even used it once but this I 
didn't get to this solution. For most other developers it'll be very hard to 
find the QSignalMapper class as it's a rarely used and very small class. So 
an aboslute minimum would be to mention this possible solution somewhere in 
the docs.

> > Yes, the first example is more lines of code, but it's more readable
> > IMHO and it's easier to analyze if you see the code for the first
> > time. 

btw imho this is more readable:

 for (int i = 0; i < 10; i++) {
     (void)new KAction(i18n("Select Group %1").arg(i == 0 ? 10 : i),-
                       Qt::Key_0 + i, this,
                       SLOT(slotSelectGroup(int)), actionCollection(),
                       QString("select_group {%1}").arg(i));
        (void)new KAction(i18n("Create Group %1").arg(i == 0 ? 10 : i),
                       Qt::CTRL + Qt::Key_0 + i, this,
                       SLOT(slotCreateGroup(int)), actionCollection(),
                       QString("create_group {%1}").arg(i));
 }

than that:

QSignalMapper* selectMapper = new QSignalMapper(this);
QSignalMapper* createMapper = new QSignalMapper(this);
connect(selectMapper, SIGNAL(mapped(int)), this, SLOT(slotSelectGroup(int));
connect(createMapper, SIGNAL(mapped(int)), this, SLOT(slotCreateGroup(int));
 for (int i = 0; i < 10; i++) {
     KAction* a = new KAction(i18n("Select Group %1").arg(i == 0 ? 10 : i),-
                       Qt::Key_0 + i, selectMapper,
                       SLOT(map()), actionCollection(),
                       QString("select_group_%1").arg(i));
     selectMapper->setMapping(a, i);
     a = new KAction(i18n("Create Group %1").arg(i == 0 ? 10 : i),
                       Qt::CTRL + Qt::Key_0 + i, createMapper,
                       SLOT(map()), actionCollection(),
                       QString("create_group_%1").arg(i));
     createMapper->setMapping(a, i);
 }

[...]
> > Unless I completely miss the point I vote for reverting the change
> > to the KDE 3.0 API. I'd be happy to take those steps and fix the
> > applications relying on the magic object-name{42} behaviour, in case
> > there is an agreement.
>
> Had I been aware of QSignalMapper, I would have suggested that to the games
> coders originally instead of the current solution.  (It doesn't fail under
> any circumstances where the previous kaction would have worked, though.)
> I've CC'd this to one of the game developers so that he can give his input.

After all I'd prefer the KAction solution since it makes the application code 
more readable and is more comfortable to an application programmer. But I'm 
fine with the QSignalMapper solution as long as
a) it happens now (i.e. not in two months or so)
b) someone changes http://edu.kde.org/developer/kaction.phtml which covers 
exactly those problems as they are very common to games (and obviously for 
edu, too as the doc was catched by them)

> Cheers,
> Ellis

CU
Andi