Re: Redesign listeners using templates (issue 235790043 by address@hidden)

[nine . fierce . ballads]

I have reviewed the changes to smob and listener.


https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh
File lily/include/listener.hh (left):

https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#oldcode84
lily/include/listener.hh:84: Listener (Listener const &other);
So you're OK with the compiler-generated copy constructor?  (Just
checking.)

https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh
File lily/include/listener.hh (right):

https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#newcode111
lily/include/listener.hh:111: return *Listener::unsmob (a) ==
*Listener::unsmob (b)
If there are no concerns about dereferencing null pointers, a short
comment would be comforting.

https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#newcode153
lily/include/listener.hh:153: T *t = dynamic_cast<T*> (T::unsmob
(target));
This is not a problem with your change, but I notice that using
dynamic_cast after unsmob is very common.  This looks like a candidate
for future simplification something along these lines:

template <class T>
static inline T* ly_unsmob(SCM s)
{
    return dynamic_cast<T*>(T::unsmob(s));
}

invoked as T* t = ly_unsmob<T>(target);

https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#newcode164
lily/include/listener.hh:164: unsmob (self)->trampoline_ (target, ev);
I suppose that here, unsmob (self) will always succeed because guile
would not have found this procedure if the type were wrong?

https://codereview.appspot.com/235790043/