cpp-rotor icon indicating copy to clipboard operation
cpp-rotor copied to clipboard

Remove calls to base::function() in overridden methods

Open gamagan opened this issue 4 years ago • 3 comments

Looking at the interface for actors, it would be nice to remove the noise from the overridden methods, so that the code only deals with its own logic. It's cleaner that way.

Instead of this:

void on_start() noexcept override {
    rotor::actor_base_t::on_start();
    ...
}

Have this, to eliminate the need to call rotor::actor_base_t::on_start():

class actor_base {
    void base_on_start() {
        // do base processing
        // ...
       on_start(); // Do overriding stuff.
    }

    virtual void on_start() {}
};

gamagan avatar Nov 11 '20 21:11 gamagan

Hi there, thank you for your proposal.

There are drawbacks on your proposal:

  • it leads to "namespace pollution", i.e. additional methods are injected, but they are not meant to be used by end-users.
  • historically, there was a need to "block" / postpone by a user some methods, with the proposed solution there is be no need of that right now. However in future, the situation might be changed again, and there will be need to change API, again.
  • right now there are some actions which are needed to be executed at different places of hierarchy, i.e. upon shutdown_finish actor_base_t should just changes actor state, in asio supervisor it optionally releases IO guard, if it was set. Surely, it is solvable via having base_shutdown_finish method as virtual... but I find it a little bit messy, if there will 2 similar methods, one for user, and one for internals.

So, now I consider the drawbacks overweight the benefits.

basiliscos avatar Nov 12 '20 17:11 basiliscos

There is no "namespace pollution", because the method is not in a place where it would conflict with a user's own method. ie, the global namespace. A base_* method would be a legitimate implementation method which user should not be calling. It could be made private, accessible to implementation callers via friendship, if it was necessary.

I fundamentally disagree with the design that requires overridden methods to call implementation methods. The overridden methods should just do work that pertain to them, not work that should be performed by the implementation. But, it's your codebase and I'm just an observer, so you do you.

gamagan avatar Nov 12 '20 23:11 gamagan

Thank you anyway, for your opinion. I'll keep the issue, and may be will apply it later, i.e. after some code stabilization/a few releases.

basiliscos avatar Nov 13 '20 05:11 basiliscos