Code review comment for lp:~dandrader/nux/geis

> > On 06/15/2012 01:34 PM, Daniel d'Andrada wrote:
> > >> On 06/15/2012 11:48 AM, Daniel d'Andrada wrote:
> > >>>>>> * GET_LATEST_EVENT should be a real function, not a macro. Use
> > anonymous
> > >>>>>> namespaces or static functions.
> > >>>>>
> > >>>>> That won't work because that code is called from both a const and a
> > >> regular
> > >>>>> method that return a const reference and a regular one respectively.
> The
> > >>>> only
> > >>>>> other way I see would be to call the const version from withing the
> non-
> > >>>> const
> > >>>>> one and doing a const_cast on the return value. But I don't see it as
> a
> > >>>> better
> > >>>>> option.
> > >>>>
> > >>>> I guess I'm confused. Why do you need a const version of the method?
> > >>>
> > >>> Because it's called from within other const methods.
> > >>>
> > >>>> Why not just return a non-const version, and any callers that want to
> > keep
> > >> it as const
> > >>>> can just assign it to add const-ness.
> > >>>
> > >>> I didn't understand you. Could you please write down some example code?
> > >>
> > >> Sorry, I realize there are a few things going on, including me
> > >> forgetting about const methods only being able to call const methods.
> > >>
> > >> I think we can resolve all this by removing the non-const
> > >> GetLatestEvent() method and making GestureEvent::Accept() and
> > >> GestureEvent::Reject() const methods too. There's nothing in those
> > >> methods that modify the GestureEvent object.
> > >
> > > That works but makes for a weird API. Accept() and Reject() doesn't sound
> > like methods that should be const (like getters) even though in their
> current
> > implementation they can be so.
> >
> > I understand the logic, but c++ constness is really an API construct,
> > not a logic construct. Usually the domains overlap, but not always.
> > Admittedly, I don't have any good examples, but I feel being true to the
> > execution when using constness is best.
> >
> > Please enlighten me if you think I'm wrong. I'm not an expert on C++, I
> > just hope I know how to not shoot my foot with it :).
>
> Changed from the macro to the const_cast way that people say "Effective C++"
> book advocates.
> Hope you like it better :)

It's better, but I'm not sure it's 100% correct. It feels to me like this shows the design isn't proper, but I admit to having written similar things in the past because I couldn't figure out how to do it "correctly" either. I certainly don't have any better suggestions, so it's good enough for me :).

Everything else I've noted has been taken care of, so I approve :). Yay!