Code review comment for lp:~alan-griffiths/mir/even-NullWindowManager-configures-surface

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I am OK providing an (almost) null implementation for the use cases you
> mention. I would prefer it if it was not possible to inherit from this class,
> though.

Why? It is entirely possible for a responsible user to specialize the class successfully.

> To use the SkeletonWindowManager class as a base effectively, especially the
> methods that have a non-null implementation, the user needs to know what that
> class method do and do not do, which may change between releases, and I think
> this is a bigger burden for the user than just having to know that, e.g.,
> set_surface_attribute() needs to configure the surface.

I don't think that is true. If overriding from a concrete class one should (with rare exceptions) call the overridden function from the override. For the user to assume that it is null omitting the call is a mistake, but not one we should be protecting them from.

> How about making SkeletonWindowManager "final", or perhaps a function
> "shared_ptr<WindowManager> make_skeleton_window_manager()" (and make the class
> internal)?

Personally, I'd only ever declare a class or method final if overriding were automatically an error (or, somehow, violates a design contract). But I could live with it.

« Back to merge proposal