[patch] KIconLoader: queryIcons() and queryIconsByContext()

Luke Sandell ls65594 at appstate.edu
Fri May 5 16:01:58 BST 2006


Michael Pyne wrote:

> I like the idea, but I don't like the way it's implemented.  It's like
> adding
> a "no, I'm being an idiot on purpose" flag to a public API. 

Actually removeIconExtension() is currently a private member function. If
you want to make it public, and I think that it useful enough that it
should be public, then I can see your point (actually it should be static
as well). However, if this is the case, then we'd better not have it print
warnings to the user (after all, they know their icon has an extension,
that is why they're calling the function). Printing the warning from that
function was a matter of avoiding code duplication, but it should be moved
into removeIconExtensionInternal() as you say.

> We would then be able to transition the internal uses of
> removeIconExtension() in queryIcons() to the
> internalRemoveIconExtension(), and there would be no code duplication.

Actually queryIcons() and queryIconsByContext() would still use
removeIconExtension() (no warnings) whereas the all internal calls would
use removeIconExtensionInternal() (warnings).

> Any better ideas?  If you don't want to do it, Luke, I'll work on a patch
> sometime this week.

If you agree with my points, I will make a patch.

-Luke Sandell





More information about the kde-core-devel mailing list