Code review comment for lp:~compiz-team/compiz/compiz.fix_1063617.7

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

On Dec 4, 2012 11:59 AM, "Daniel van Vugt" <email address hidden>
wrote:
>
> Review: Needs Information
>
> 1. As usual, I find it unreadable due to the long lines, from long
identifiers and template use everywhere.
>

Can you identify which lines you find problematic, and provide alternative
solutions please?

> 2. Can you find a better name for: class
RequireSettingInterfaceRedirection ? There seems to be no (strong enough)
noun really describing what an instance of such a thing is. How do I
visualize a RequireSettingInterfaceRedirection in my model of the world? Is
"redirection" the noun? Sounds more like an operation so would be a method
of some other class.

Its meant to be a state that automatically cleans up. Hence the reason why
its a class.

Its only really used once though, so I suppose it can be changed to:

->RedirectGetTypeFunc ()
->DoThing ()
->UnredirectGetTypeFunc ()

>
>
> --
>
https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1063617.7/+merge/136917
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

« Back to merge proposal