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

Alan Griffiths (alan-griffiths) wrote :

I'm concerned that the code still looks "fragile" (to use Sam's word). For example:

50 + y1 = geom.height () + geom.border ();
...
59 + y2 = geom.height () - geom.border ();

It isn't explicit why border is added in one case and subtracted in the other. I suspect that there's an abstraction missing (or at least suitably named functions that derive the appropriate result from geom). Something like "internalHeight = internalHeight(geom)" is much easier to follow.

I've not tested the latest version enough to be confident in it. Will aim to do so on Thursday.

« Back to merge proposal