[Merge] lp:~ssweeny/location-service/delayed-providers.15.04 into lp:location-service/15.04
Alberto Mardegan
alberto.mardegan at canonical.com
Mon Nov 9 10:56:21 UTC 2015
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.
--
https://code.launchpad.net/~ssweeny/location-service/delayed-providers.15.04/+merge/275384
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service/15.04.
More information about the Ubuntu-reviews
mailing list