KActionCollection: it's a jungle

[Andreas Hartmetz]

Am Montag, 19. März 2007 08:27:06 schrieb Hamish Rodda:
> On Sunday 18 March 2007 10:16, Andreas Hartmetz wrote:
> <snip>
>
> > Therefore, I propose to remove all of these:
> >
> > * addDocCollection
> >  a)huh?, b) used in one case in KHelpCenter. Can't be that useful.
>
> Discussed already in the thread.
>
> > * setDefaultShortcutContext:
> >  used three times, to set it to the default each time...
> > * applyDefaultShortcutContext:
> >  a)wtf?, b) unused
> > * defaultShortcutContext:
> >  then obsolete
>
> These functions are to prevent having to loop over each action and change
> its shortcut context, or to set each by hand when setting up your actions. 
> I exposed them because they were needed by the internal api
> (*AssociatedWidget*) and seemed to me like useful functions for 3rd party
> code to access.
>
> However I don't mind their being made private if you feel so moved.

It really doesn't hurt to have some more bytes in the library, but utility 
functions that save one line of code (direct call vs. foreach) are IMHO not 
worth the increased mental effort when reading the documentation.
Partly because of increased length, partly because you start thinking "why is 
it there, does it really only replace a foreach loop, what did I miss?".
A foreach here and there adds more bytes to the applications, and for me it 
makes sense to add tiny utility functions if and only if they will be used 
*really* often.
lxr did not find any use so I just don't think they reach a level of use to 
justify them.
>
> > * forgetEnabled:
> >  a)wtf?, b)unused.
> > I'm unsure about setEnabled/isEnabled... it's hard to find them because
> > the name occurs so often and I'm still investigating with a massive grep
> > orgy.
>
> I think I added these, sounded like a useful feature again to me.  They
> could go, if you're happy to change all uses of them.  I don't see the need
> to remove them, personally.
>
<strikeout>
They are going to stay, one reason being that I can't think of a reliable way 
to find all uses of them without using the "see if anything breaks in any KDE 
module when they are removed" approach.
The other reason is that they are useful anytime you are in the process of 
building a collection with an occasional run of the event loop in between. 
Large applications can work that way.
forgetEnabled - IMHO it should be called stopEnforcingEnabled or similar. The 
current name sounds like it does the opposite of what it actually does. Is 
this a non-native speaker's problem?
</strikeout>
------------------
No wait, a better solution would be doDisableOnAdd(bool) / 
doesDisableOnAdd() / allSetEnabledTo(bool enable). Actions are enabled by 
default (correct?), so no reason to have them enabled when adding them.
doesDisableOnAdd() would probably only help with debugging.

This set of function could do essentially the same, but in a *much* less 
confusing way. isEnabled() would have to be dropped - what does it mean 
anyway for a collection? At least one is enabled? All are enabled? None is 
very useful, because single actions can differ for various reasons.
Are the function names OK with native speakers?

> > * *AssociatedWidget(s):
> >  why should a collection ever have this?
>
> Already discussed, replaces KAccel functionality iirc.
>
> Cheers,
> Hamish.

Hello Hamish! I hope that you didn't take the criticism too personally... the 
class looked to me like like a straight port / merge with any old sins still 
in there.