Code review comment for lp:~vanvugt/unity/fix-918329-trunk

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

On Fri, 20 Jan 2012, Daniel van Vugt wrote:

> Thanks.
>
> And you're right about static casts instead of dynamic. But this version avoids all casts which is even better I think.

Yes, boost::static_pointer_cast has the cost of the boost::shared_ptr
constructor (which is not that much, but something to dislike anyways).

I don't know why the static pointer casts were changed to dynamic casts.
That seems excessively silly, especially considering that one of them was
in a paint function (!). However, use of downcasting usually indicates a
problem with the design, one that should be revisited later (also related
to testing, since the base class exists for testing purposes)

> --
> https://code.launchpad.net/~vanvugt/unity/fix-918329-trunk/+merge/89391
> You are reviewing the proposed merge of lp:~vanvugt/unity/fix-918329-trunk into lp:unity.
>

« Back to merge proposal