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

Do not prevent subclassing your libraries. #23

Closed
wants to merge 1 commit into from

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Oct 25, 2019

No description provided.

@tomlankhorst
Copy link
Contributor

Virtual functions have a non-zero performance impact which might be undesirable for a piece of code like an algorithm.
A class might resort to composition to extend the functionality of Algorithm, it should then rely on the publicly exposed functions of Algorithm. I'm not a big fan of protected functions since functions should either be part of the interface (public) or not (private).

@peci1
Copy link
Contributor Author

peci1 commented Mar 27, 2020

You're right virtual functions might be costly if the function is called often. Maybe the code could have a non-virtual function OftenCalledFunction and a virtual NotSoOftenCalledFunction. But that decision is on you.

As for protected members - composition is sometimes inefficient. Imagine a class that has some data protected by a mutex. And it has public fuction getData() and private getDataNoLock(). If you want to extend the functionality and the latter function is private, you have no other way than to call the locking variant every time. Even though you might either pretty well know when locking is needed, or you might even hold the lock (if it's made protected, too). This is something composition has no answer for.

@tomlankhorst
Copy link
Contributor

Thank you for your reply and suggestions.

I do agree with you that there are situations imaginable where composition is challenging.

However, say there exists a locking public function getData() and there is a non-locking variant getDataNoLock() in class A:

  • If getDataNoLock() is private, then this function is not intended to be used outside of A.
  • If getDataNoLock() is protected, the function can be used in any class that extends A. That effectively means that the function is accessible outside of the class and therefore part of the class' interface. So, then why not make the function public in the first place?

If you control both class A and the extending class B, you could also resort to friend declarations.

Now, I do consider adding an example where inheritance is more appropriate, but this algorithm is probably not it.

@peci1
Copy link
Contributor Author

peci1 commented Mar 27, 2020

You're right, there are always ways. Making functions protected is (from my point of view) more a safety feature "against" users of the class. Or in other words, there is "user API" (public functions) and "developer API" (protected functions). By using public and protected you can nicely tell these two apart.

Friend declarations are something that I really don't like, because they break all kinds of contracts. The only place where I like using them is testing. But even there I like to keep the base class "clean" of friend declarations, and in the test, I subclass it, and add the tests to be friends of this subclass. Again, when things are private, you can't do it this way and would have to clutter the base class with totally irrelevant friends.

But that's all just my viewpoint. I'm aware that some best practices go in the direction you present.

I support the idea of adding another example for inheritance and keeping algorithm as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add example where virtual call is appropriate
2 participants