A very simple solution, which comes at no cost to SFML and greatly increases it's functionality, is for SFML classes to change their private contents and functions, to protectedActually, there is one major hidden cost with this approach: everything that is accessible from outside SFML implementation code is de facto part of the *public* application interface. Hence, `protected` is somewhat equivalent to `public` meaning that we cannot change them without potentially breaking users' code. From a maintenance point of view, the modification you're proposing has huge drawbacks.
In regularly using SFML, I often encounter a class where I want to expand it's basic functionality (a term for this is extensibility), but I'm unable to do so as it's contents and functions are declared private, which means only members of that class and friend classes can access it.Yes, and this exactly the purpose of encapsulation. protected is almost public, because it exposes implementation details, and by doing so, tremenously limits the implementation.
A prime example would be adding pause functionality to the sf::Clock.It's indeed a great example, because it can be perfectly extended without inheritance, see thor::StopWatch (http://www.bromeon.ch/libraries/thor/v2.0/doc/classthor_1_1_stop_watch.html).
A very simple solution, which comes at no cost to SFML and greatly increases it's functionality, is for SFML classes to change their private contents and functions, to protectedActually, there is one major hidden cost with this approach: everything that is accessible from outside SFML implementation code is de facto part of the *public* application interface. Hence, `protected` is somewhat equivalent to `public` meaning that we cannot change them without potentially breaking users' code. From a maintenance point of view, the modification you're proposing has huge drawbacks.
Now, I'm not saying that no function should be protected instead of private but this should be discussed on a case-basis.
Yes, and this exactly the purpose of encapsulation. protected is almost public, because it exposes implementation details, and by doing so, tremenously limits the implementation.
This is the typical Java way of thinking, where classes are the core language element and deeply nested class hierarchies are the standard. Not so in C++ -- here, you have many possibilities of extending functionality, and inheritance is usually not the best option.
It's indeed a great example, because it can be perfectly extended without inheritance, see thor::StopWatch (http://www.bromeon.ch/libraries/thor/v2.0/doc/classthor_1_1_stop_watch.html).
You make a common mistake: you violate the Liskov Substitution Principle by assuming that stopwatches are clocks and have every property of their base class. Currently, a class invariant of sf::Clock is to be always running and to count the time since the last reset() call. Depending on the semantics you provide, reset() would need to be virtual, or you have a broken base class. The destructor would also have to be virtual, for people who destroy stopwatches polymorphically. This would add useless overhead that most people never use, but all people have to bear.
And this is typical: C++ classes must be explicitly designed as base classes, inheritance can't be an afterthought.
I can widely agree with Hiura and therocode. If you find a certain component in SFML lacking, you should report this to us, and not just patch your way around it. This way, other users can benefit from improvements as well.
I don't think you actually looked at Thor's Stopwatch. It internally uses an sf::Clock.It's indeed a great example, because it can be perfectly extended without inheritance, see thor::StopWatch (http://www.bromeon.ch/libraries/thor/v2.0/doc/classthor_1_1_stop_watch.html).By rewriting an entirely arbitrary class that makes use of system clock calls... which makes sf::Clock now redundant to me. sf::Clock has all the components to become a pauseable clock, but it requires I have access to it's stored time variable from a sub-class perspective.
Stopwatch would inherit from ClockYou are suggesting that a stopwatch is a clock, which is incorrect.
I don't think you actually looked at Thor's Stopwatch. It internally uses an sf::Clock.
You are suggesting that a stopwatch is a clock, which is incorrect.
Having protected allows them to extend their own limited use case to that class without having to bug Laurent to add it in.On the contrary, after this private -> protected modification we are always bothered with those additional functions, which would now be part of the API but were previously implementation details, when we have to fix a bug or add a new feature.
Have you looked at line 83 (http://www.bromeon.ch/libraries/thor/v2.0/doc/_stop_watch_8hpp_source.html)? ::)I don't think you actually looked at Thor's Stopwatch. It internally uses an sf::Clock.
That's not evident from the HPP supplied. Perhaps in the CPP it is, but that's not been provided.
You are suggesting that a stopwatch is a clock, which is incorrect.
Then how does it keep tabs on the transition on time if there is no clock intrinsic?
On the contrary, after this private -> protected modification we are always bothered with those additional functions, which would now be part of the API but were previously implementation details, when we have to fix a bug or add a new feature.
Have you looked at line 83 (http://www.bromeon.ch/libraries/thor/v2.0/doc/_stop_watch_8hpp_source.html)? ::)
There's a big difference between the is-a and the has-a relationship. You should read a bit on that and especially about the LSP as Nexus said. ;)
What additional functions would you be bothered by? I don't understand the problem you guys are referring toThe ones that were changed from private to protected.
You make a common mistake: you violate the Liskov Substitution Principle by assuming that stopwatches are clocks and have every property of their base class. Currently, a class invariant of sf::Clock is to be always running and to count the time since the last reset() call.
Inheriting your "pausable clock" (i.e. stopwatch") from an sf::Clock means that you are saying that your stopwatch is a "clock that only keeps track of time passed with an ability to restart", since that is what an sf::Clock is.
Nexus mentioned this already:You make a common mistake: you violate the Liskov Substitution Principle by assuming that stopwatches are clocks and have every property of their base class. Currently, a class invariant of sf::Clock is to be always running and to count the time since the last reset() call.
I don't see why it's so important to inherit from sf::Clock rather than to just use sf::Clock.
The ones that were changed from private to protected.
As soon as the function becomes protected it becomes callable by user code which means SFML can no longer freely change the implementation since that might break user code that calls it and depends on details of the previous implementation.
For me, if a class calls a duplicate function of another class (the stopwatch example you've given calls 'getElapsedTime' - which is a duplicate function of sf::Clock), I feel that's a signpost saying 'this class needs to be a subclass', especially if it shares variables and merely 'adds additional functions'.
If I pass my AdvancedClock (to you, a type of stopwatch) to a function, the function can still treat it as though a clock - because it still has those types of variables and functions in-place: getElapsedTime, restart etc.
Ah, there, a valid response to the actual topic.Which is just a rephrasing of what I said in the first answer to your proposal...
This is an invalid rebuttal, however, because SFML has often changed things that go on to break user's code - for example, shifting the naming convention from UpperCase style to lowerUpperCase style, the removal of rand etc. I think generally, as a rule, when one uses a codebase under development, they have to expect code breaking changes.Your reasoning is flawed in two places at least: a) you're comparing SFML 1 and SFML 2 -- like you could be comparing apples and oranges, this is wrong -- and b) SFML 2's API is stable -- which means the user can expect his/her code to remain valid until he updates to SFML 3.
I think not wanting to change something because it might break code isn't legitimate here