Code review comment for lp:~ssweeny/location-service/delayed-providers.15.04

I'm coming from a programming style that keeps virtual methods to a minimum, and I do understand if that might not be of your liking, so feel free to discard this comment. :-)

Wouldn't it be possible to implement the same without breaking the ABI?

At least, I see no reason why these methods need to be virtual:

    virtual core::Signal<bool>& boot_state_changed();
    virtual const core::Signal<bool>& boot_state_changed() const;

given that you are providing a default implementation that I don't see any reason to override. As for the getter:

    virtual BootState boot_state() const;

this could be made non virtual by adding a protected method like

    void set_boot_state(BootState state);

that subclasses can call to set the boot state. This method would also take care of emitting the boot_state_changed() signal.

The only method for which a virtual declaration makes a lot of sense is request_boot(), but OTOH I wonder if we could remove it altogether: we could just specify that boot should start when the provider's constructor is invoked, or (if we really need a two-phase initialization) when the

    virtual const Controller::Ptr& state_controller() const;

method is first invoked (it's a virtual method, so the subclass can use it to start the initialization). I can see that purists might not like to have this method be overloaded with the booting role too, but it's something that can make sense, if clearly explained in a doc comment.

It all depends on whether preserving ABI is a goal or not.