On Sun, 15 Apr 2012 at 23:09:39 +0200, Rodolfo García Peñas wrote:
> The new StrConcatDot function can be used now in wdefaults.c
>  
> -     if (instance && class) {
> -             char *buffer;
> -             buffer = wmalloc(strlen(instance) + strlen(class) + 2);
> -             sprintf(buffer, "%s.%s", instance, class);
> +     if (instance || class) {
> +             buffer = StrConcatDot(instance, class, False);
>               key = WMCreatePLString(buffer);
>               wfree(buffer);
> -     } else if (instance) {
> -             key = WMCreatePLString(instance);
> -     } else if (class) {
> -             key = WMCreatePLString(class);
>       } else {
>               key = WMRetainPropList(AnyWindow);
>       }

Code must be easily understandable and the old one was easier
to follow. 

I don't care if the end result is theoretically the same by playing
games with 'alwayspoint' and && versus ||. The resulting code is harder
to follow after your patch. The old code with the else ifs was plain
obvious - you can immediately tell what 'key' will be in the various
situations whereas after your patch that is no longer true.

The cleanup worth doing in the above code is to replace

-               char *buffer;
-               buffer = wmalloc(strlen(instance) + strlen(class) + 2);
-               sprintf(buffer, "%s.%s", instance, class);

by

buffer = StrConcatDot(instance, class);

I will remove all patches regarding StrConcatDot() from #next, sorry.


-- 
To unsubscribe, send mail to [email protected].

Reply via email to