On Sun, 24 Mar 2013 at 23:57:40 +0100, Daniel Déchelotte wrote: > > [...] > > So I think this function should have another parameter, say an > > integer (or even an boolen) called 'combine'. [...] > > For a brand new function, yes, probably.
But you are adding a new function with this patch. This is the time to have it in optimal form. > Here, I'm not sure it's worth reworking that old function. Which 'old' function? It may be old for you, but not for us :-) And with this patch you are definitely reworking the old duplicated function(s) currently in wmaker's code and substituting by your 'new' consolidaded function, aren't you? > To improve readability, we could repeat the comment from WPrefs.h > and/or create a local variable combine = (icon2 != NULL), that the > compiler will be free to optimize out. I think having the extra parameter 'combine' is much cleaner than playing games with icon2 being NULL or not. C'mon, just the name of the variable being 'icon2' should immediately tell you that you have to use it as an icon. Using it as a parameter to make extra decisions is plain wrong (or confusing and not straightforward), whether documented or not. -- To unsubscribe, send mail to [email protected].
