Code review comment for lp:~smspillaz/compiz-core/compiz-core.fix_931927

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

170 + CompString & emptyString ()

Rather invites:

emptyString () = CompString("Murphy rules!");

(The same applies to a lot of the functions, and the original [non-]constants.)

So, we should add const to the "constants" while addressing this problem.

On the comment discussion: Global data, monostate & singleton are all likely causes for issues. So this code could do with some cleanup regardless of "on demand" or "global data".

As Sam says, it is slightly safer to use "on demand" than global variables - as it ensures they have actually been initialized before use. OTOH, if we "know" there is no init-sequence issue that only makes the design marginally better.

So, I wouldn't have done the "wrapping" if the problem can be fixed more simply. But, if we had the code ready to go I'd take it, it make the codebase a safer place to live.

review: Needs Fixing

« Back to merge proposal