The breaking code argument is invalid to me. Considering the amount of changes that might occur, you just can't expect your code to still build after anyway.I built some one year old code this week and I'm using my LuaConsole that is from like 2014 originally, I don't see why it shouldn't build, it's a simple game and tool (routing game and my simple console) but still... SFML 2.x has an API as stable as a rock, I can't recall any problems building either.
The first, as wildly known, one should not help the compiler by writing code in a particular way. The same rule applies to analysers.This only applies to the context of optimizations. When it comes to the matter of correctness, whether we specify a function with override or mark a function as deleted, it is not a matter of who does it better, but of what we want to tell the compiler. We speak to the compiler in terms of syntax and the C++ language, not English or inheritance relations. Moving functions to have private accessibility (and importantly this is not the same as visibility) so they can't be accessed outside the class is not equivalent to marking them as deleted thus preventing the compiler from generating them at all. We aren't "helping" the compiler here. In either case we are telling it exactly what it should do, the "what" is what differs.
The second, when processing the AST of a given C++ class, because the language is statically typed, the parent classes, if any, are known and present in the AST. Hence, it makes no difference whatsoever which technique is applied here for analysis tools.This is assuming that analysers will exhaustively scan the AST to make sure it didn't miss anything. Just like optimizations, analysis isn't governed by any specification. Implementers are free to do what they think serves their userbase the best. If they need to employ heuristics that only cover 99% of all cases to keep the complexity of the task in the realm of the feasible, they will also do so. I'm not saying that there can't be tools that will understand the inheritance relation as well, we just have a higher chance of getting tools to "do the right thing" if we keep the AST as simple as possible. Inheriting from sf::NonCopyable multiple times as is the case with some SFML classes only contributes to unnecessary complexity that might turn out to be a disadvantage in some cases.
Is it really reinventing the wheel? To me it looks more like a way to be more expressive, with a lighter cognitive load. Let's not forget that we are not only talking about copy operations, but also move operations.The factors that determine the true cognitive load differ from person to person in part based on past experience as well. What I can say is that for a hypothetical person, who has just learned C++ post-11 and never seen this non-copyable base class pattern before, it will definitely incur a higher cognitive load because they unarguably have to learn something new in addition to what they have learned as part of the language itself. sf::NonCopyable is optimizing expressiveness in favour of "experienced" people who recognize the pattern from pre-11 C++, whereas = delete is optimizing expressiveness in favour of "novice" people who are only familiar with the standard post-11 C++ idioms employed in the STL. As I've stated before, one way or the other, C++ developers whether they are new or experienced will have to deal with the way the STL is specified. Forcing them to learn something else in addition to that because we follow other philosophies doesn't make SFML seem like a "native" C++ library for people who expected adherence to principles they know from elsewhere (such as the STL).
In the first case, i.e. without NonCopyable, one has to add a lot of boilerplate to the Resource class. In 5 lines, the class name is repeated 9 times. Reading this *is* expensive. And if we have a second resource class we need to do it all over again.I generally don't tend to measure the cost of reading code by the repetitiveness of certain tokens. If instead of the class name the signatures would contain 9 different tokens, it would be just the same for me. Considering some of SFML's function signatures can get quite long and number of member functions can grow quite large, I don't think it makes sense to judge the way something is implemented based among other things on lines of code or the length of those lines. When I read these lines as a human, my brain recognizes patterns, beginning with the = delete at the end. When I see 4 of them, I can stop reading because I know the class is neither copyable nor moveable. If I see only 2 of them, I look for double ampersands. If I don't find any, I know the class is non-copyable but movable. Maybe it's just the way I've learned to read code, but surprisingly, I've noticed that when I try to understand code, it doesn't boil down to how much is written, but often how it is written. In this aspect, my brain prefers longer, more regular patterns over shorter "non-patterns" regardless of spoken language.
In the second case, the boilerplate is moved to a confined place and not repeated across the code base. The fact that the resource cannot be copied is clear right from the first line of the class if you take the time to understand why the word non-copyable means -- i.e. it's not even about C++ but about plain English.We would be putting all our eggs in one basket by requiring it to be evident from the first line of the class whether a class is copyable or not. The class won't be copyable even when not inheriting from sf::NonCopyable as soon as one of its members is non-copyable, either through sf::NonCopyable or some STL class that ultimately uses = delete. All I'm saying is that the argument that it is obvious with sf::NonCopyable cannot be made for the general case. There are cases where it will be just as "unobvious" as = delete, and like I just said, if you have classes with both sf::NonCopyable and non-copyable STL classes as members, the compiler will be the last one to care about not confusing the developer with multiple differing error messages.
You might argue, rightly, that one should not need to look at the source code itself but only at the documentation. But the generated documentation is very clear too: it appears at the top of the page with the composition graph and again in the list of functions. Plus, as you said, in any case we need to properly document how the class is intended to be used. Bottomline, it's a win-win.This is just a matter of where doxygen places the inheritance graph. I must admit, the first time I saw doxygen documentation I also asked myself why the diagram was placed before the member list let alone the synopsis. The inheritance relation for me is part implementation part interface specification. In any case, it really shouldn't take precedence over a thorough description of what the class is actually supposed to be good for. If while reading the description one realizes that the class isn't really the thing they need, they wouldn't have to bother understanding what it inherits from. The graph itself is incomplete information and as such for me doesn't really belong where it currently does, but after so long I guess I just got used to it (if I even bother to look at it for more than 1 second).
Invoking that *some* users ask why they can't copy some objects is not that relevant: this part of the developer population is obviously not used to read the documentation/compiler errors, or simply are beginners. They'll ask the same questions about = delete.Maybe they might, but we can't blame them for coming to us without doing some research on their own when we don't even make sure they get error messages they can easily search for. I didn't check, but I have a feeling you will get more hits regarding "is explicitly marked as deleted" as opposed to "sf::NonCopyable::operator= is private in this context".
As you said, we have to properly document this, but with NonCopyable we get for free the = delete in the inherited functions list.Again, I haven't checked, but what I noticed is that inherited constructors or assignment operators aren't displayed in the derived class. Whether or not deleted functions are even listed is another question altogether.
Arguing that the C++ committee didn't include std::non_copyable implies it is not something we need/want is also an irrelevant argument IMO. It's clear that the committee hasn't always taken the right decisions. But most importantly, they don't need to solve every issues and don't want to impose too many design choices on the users.But in this case, this is an issue that they spent their time solving, with a more or less concrete solution. The inclusion of smart pointers into the standard also doesn't prevent us from still using the boost versions either. It's just a matter of convenience. Yes... I can imagine there were people on the committee that might have been opposed to this idea, but so is the nature of design by committee. They might not always make the right decisions, but when decisions are made, they become part of the language standard, the standard everybody has to adhere to especially a library written in C++. We can resist it if we want, but in that case we should refrain from sending people off to learn modern C++ only to tell them after they are done that we don't find everything we told them to learn that great either.
Moving functions to have private accessibility (and importantly this is not the same as visibility) so they can't be accessed outside the class is not equivalent to marking them as deleted thus preventing the compiler from generating them at all. We aren't "helping" the compiler here. In either case we are telling it exactly what it should do, the "what" is what differs.I think there's a slight misunderstanding: I'm not arguing against = delete, I'm arguing we should keep NonCopyable. Using = delete as an implementation details in NonCopyable is obviously the right decision.
This is assuming that analysers will exhaustively scan the AST to make sure it didn't miss anything.
The factors that determine the true cognitive load differ from person to person in part based on past experience as well.
As for move operations, do you propose we introduce a sf::NonMoveable as well?At the present time, I see no reason to do that. Which class should not be moveable?
I generally don't tend to measure the cost of reading code by the repetitiveness of certain tokens. If instead of the class name the signatures would contain 9 different tokens, it would be just the same for me.I'm happy for you that you can deal with this very efficiently, but I'm not convince it's the case for the majority of human beings. I mean, if it weren't the case project such as https://github.com/tonsky/FiraCode wouldn't be needed. It's also what `auto` is (partially) about: reducing to what really matters. You can also find many improvements on the syntax, especially with functional languages, where a lighter syntax -- and therefore a lighter cognitive load -- allows to improve expressiveness.
We would be putting all our eggs in one basket by requiring it to be evident from the first line of the class whether a class is copyable or not.If we were to assume one doesn't need to read the class documentation, then yes. But I think we agree that this assumption is incorrect. So I see no issue here.
I have a feeling you will get more hits regarding "is explicitly marked as deleted" as opposed to "sf::NonCopyable::operator= is private in this context".
Again, I haven't checked, but what I noticed is that inherited constructors or assignment operators aren't displayed in the derived class. Whether or not deleted functions are even listed is another question altogether.Not sure if it's relevant, but they do get listed as well. See the attached example where I've added some dummy ctor/op=.
We can resist it if we want, but in that case we should refrain from sending people off to learn modern C++ only to tell them after they are done that we don't find everything we told them to learn that great either.Let me disagree. ;-) People should learn what alternatives exist and then choose the one that fits best their challenge. In a way, that's what we're doing here: pondering whether it's better to do X or Y to solve P. A daily task for (C++) programmers anyway; e.g. you can use many container to solve P but some are better at A than B. Designing class abides to the same law, I'm sure you'll agree. So generally speaking, sending people to read on modern C++ is always a good thing, but they should not leave their critical opinion behind.
I think that this specific dilemma should be:I'll move the discussion to a dedicated thread when I get home, but it's exactly these kind of discussions I want to see in this sub-forum!
- Moved to separate topic. Too much OT for this sub–forum, don't you think?
- Decided through query as it is almost completely design choice and neither of solutions can be proven to be better than other one.
I'll move the discussion to a dedicated thread when I get home, but it's exactly these kind of discussions I want to see in this sub-forum!I meant that there should be no OT in this sub-forum, i.e. each thing should be discussed in proper thread (:P) while this one certainly doesn't fit 'C++ standards' name.
Not sure what you meant by query? There's no point in having a private discussion about it, however we'll have to find a way to come to a decision. But before jumping to anything, I'd like to hear the opinions of other team members.By query I referred to questionnaire/form in which qualified users would be allowed to vote for proffered design.
Class inheritance means "IS-A" not "HAS"/"HAS NOT".Public inheritance means "IS-A". Private inheritance, which is how sf::NonCopyable is meant to be used, is like composition (you inherit features, not a public interface), so it really is "HAS".
/// The type of inheritance (public or private) doesn't matter,
/// the copy constructor and assignment operator are declared private
/// in sf::NonCopyable so they will end up being inaccessible in both
/// cases.