Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
gamagan opened this issue Nov 11, 2020 · 3 comments
Open

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

gamagan opened this issue Nov 11, 2020 · 3 comments
Labels
question Further information is requested

Comments

@gamagan
Copy link

gamagan commented Nov 11, 2020

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() {}
};
@basiliscos
Copy link
Owner

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.

@gamagan
Copy link
Author

gamagan commented Nov 12, 2020

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.

@basiliscos
Copy link
Owner

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 basiliscos added the question Further information is requested label Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants