sf::Image::getPixel()/setPixel() validity

[shadowmouse]

Thank you Tank. I do think that even if asserts aren't going to be put into these functions, the documentation for them should say that the STL containers (usually) won't allow incorrect values, as at the moment it makes it seem like an unprotected function, and an assert with a message like that would be very useful in any function that has the possibility of taking incorrect values.

Oh, and for the record binary1248, thank you for taking the time to voice your opinions, but making people use a debugger where it is not necessary, purely because it is there, seems pointless. And also while I know I have nowhere near the experience of you guys, I do object to being (implicitly) called a c++ noob. I taught myself GCSE standard c++ when I was 10 (when c++ is regarded as too difficult to teach the higher achieving GCSE students), and I am currently doing A level to University level programming in year 11, while my class mates struggle with checking user inputs from a shell. And no, people should not be encouraged into languages like Java, because if people aren't encouraged away from c++, then we'll have more future c++ devs and so more programs written in good fast languages, rather than ending up with situations where brilliant programs (such as blender to give a single example), are unable to be used for large projects, because the developers feel more comfortable programming in slower 'easier' languages.

[Tank]

Blender's written in C.

[shadowmouse]

No, it uses python. Just checked on the website and it uses c, c++ and python, with the backbone being written in c and c++ and most of the functions written in python, holding your mouse over functions shows that they are all called by python functions.

[Nexus]

I fully agree with Tank here.

Asserts are for the developers of the library, not the users.

Assertions are regularly used to check preconditions, both internal or external. The STL is the best example: they do those checks for the users, not for themselves.

They were never meant to replace proper bounds checking code.

How would such code look? Other error-checking mechanisms such as exceptions and error codes are associated with a runtime cost. You don't want to have an overhead every time you access an index, because as you say, this is not Java. Assertions are the ideal compromise between safety in debug and speed in release mode.

By using a debugger, out-of-bounds errors such as this can be resolved in a matter of seconds, even with a complete stack trace.

This is simply not true. Out-of-bound errors cause undefined behavior, there's no guarantee that debuggers detect them. For example, it's very well possible that there's other data after the end of the array, and by accessing an index beyond the array, you just write to that data.

struct MyStruct
{
        int a[3];
        int b[3];
};

int main()
{
        MyStruct s = {};
        s.a[3] = 12;

        // Values after assignment:
        // s.a = {0, 0, 0}
        // s.b = {12, 0, 0}
}

That's exactly the reason why high-level containers such as std::vector and std::array provide additional index checks.

[Tank]

No, it uses python. Just checked on the website and it uses c, c++ and python, with the backbone being written in c and c++ and most of the functions written in python, holding your mouse over functions shows that they are all called by python functions.

Yes, it uses Python. For extensions and the game engine. The Python info in tooltips are for extension writers as some kind of documentation, so you know what to call in Python to reach the same functionality. The code behind those calls is implemented in C though.

Blender is everything, but not slow.

[shadowmouse]

No, it uses python. Just checked on the website and it uses c, c++ and python, with the backbone being written in c and c++ and most of the functions written in python, holding your mouse over functions shows that they are all called by python functions.

Yes, it uses Python. For extensions and the game engine. The Python info in tooltips are for extension writers as some kind of documentation, so you know what to call in Python to reach the same functionality. The code behind those calls is implemented in C though.

Blender is everything, but not slow.

Ignoring the fact that this is off topic, I was part of the first ever project (that we could find out about), where we made the school trophy by scanning people in and (me) doing the modelling in blender. It crashed every computer we put it on, including an alienware aurora, that has an i7 and 16gb of ram. It was also taking about a minute per vertex move or quadrilateral creation.

[therocode]

Ignoring the fact that this is off topic, I was part of the first ever project (that we could find out about), where we made the school trophy by scanning people in and (me) doing the modelling in blender. It crashed every computer we put it on, including an alienware aurora, that has an i7 and 16gb of ram. It was also taking about a minute per vertex move or quadrilateral creation.

I bet you that's due to implementation, not the language selection. If it was years ago, it is likely that blender back then was more unstable.

[shadowmouse]

Actually, it was a couple of months ago.

[binary1248]

Tank was right... this did escalate quickly, but only after he said it escalated quickly.

I guess it's my turn to remind people that this is the "Feature requests" sub-forum. In past feature requests it was always Laurent who ended up asking the typical "How will this benefit SFML users in general?". As soon as I do it and in the process suggest what I think is a better alternative that everybody has access to and IMHO should learn to use, everybody labels me an elitist. Just to be clear, I have no hidden agenda, I just want the best for the library, and I thought it was a valid concern considering whether adding asserts would actually have any real impact on the daily lives of SFML users. It was never my intention to point fingers at anyone in this thread. Playing devil's advocate can have such unpredictable results sometimes.

You can go on and on about how assertions are great when you use them, but remember, it's not just about you. The people who have bothered posting in this thread are probably the ones who would benefit from them the least. I myself use assertions all over my code and find them great, and yet I would never consider adding them as part of the public API of any library I write. Now I ask again: Would assertions benefit those who don't bother posting on the forum when they don't have a problem with SFML? Since we can only estimate using previous experience, I would say no. You might have another opinion, but I think yours would be harder to prove than mine taking into consideration they are both based on estimates. If there is one thing that I have learnt browsing through the forum, it is that one must never underestimate the helplessness of some of its users. Now you might say: Well, those users are better off learning to avoid common mistakes and develop good habits in the first place. This kind of disproves your own point. If assertions aren't targeted at them, and more experienced users don't make the kind of mistakes that are caught by assertions that often, then who are they targeted at?

Now for my pragmatic side.

If we try to look for cases where people have sought help on the forum because of exactly this or similar "problems", then you won't find that many. Maybe because it is so trivial that even beginners are able to spot them without external help. If you are going to add assertions every time at least one person posts with a corresponding problem, then it is no different to adding a new feature every time someone needs it in their own project. If you look at assertions like genuine "feature requests" then the same conditions for their rejection would apply here. I am just trying to treat them the same.

Also, if we went ahead and added an assertion here, we would have to add them everywhere else as well to stay consistent. I'm just estimating at this point, but this is at least a few hundred lines of new code, maybe even up to or over 1000. This adds to the total maintenance cost of the library, considering that the assertions would have to be adjusted, possibly at multiple remote locations of the library any time we decide to change any pre-/post-conditions. Is this extra maintenance cost worth that little peace of mind that assertions might give us?

Like I said, there are tools ("good" debugging environments, clang with its various sanitizers, which would have already caught Nexus' example during static analysis) that are more effective at catching programmer errors than manually maintained asserts. We shouldn't have to re-invent the debugging wheel just because we find a specific concept awesome. Leave the developer aids/tools to the ones who know what and how to implement them for the end user. Having a tool that goes hand in hand with the toolchain you are using is much more powerful than anything a library writer could ever add to the library, that is of course if people use those tools in the first place.

Lastly, we mustn't forget that like everybody has already stated, assertions only have an effect in debug configurations. Think of the times when people have come to the forum for help with an obscure screenshot that proves they didn't bother building/running in debug. Also, think of the people who make use of the SFML packages on Linux systems, they won't benefit from the assertions at all since the packages rarely if ever come with a debugging configuration. Same story if people get SFML via NuGet. If we really expect people in these environments/situations to build the library themselves or download the official release archives just to benefit from assertions, we already lost the battle.

All I am asking for at this point is tangible proof that the change we are going to make is actually going to be effective (and more effective than its alternatives) by benefiting more than 1% of the typical SFML user base. Everything posted up to now has failed to meet that requirement, talking around the actual point. Provide proof and I will happily concede.

TL;DR: Unintended self-fulfilling statement, concepts are awesome if they benefit you, concepts don't have to benefit others just because they benefit you, still no real proof this change is worth adding.

[Hiura]

I agree with the devil's advocate here. 

A crucial argument for me (it was partially covered by Binary but I'd like to put more emphasis on it): Such defense programming is good in debug mode hence why assertions are (or should be) disabled in this configuration. And when developing an application, it is my responsibility to fulfill the contracts (mostly preconditions here) imposed by the libraries I use. Using those libraries in debug mode to benefit from its assertions is not a good strategy.

From the user POV, this strategy only works properly when those assertions are properly documented in the libraries APIs. And from the implementation POV, Adding in SFML documentation that such and such functions have assertions for this and that would, as Binary explained, remove a lot of flexibility/add a lot of maintenance burden.

But beyond this technicality, using libraries' assertions is not a good strategy simply because this is no longer defensive programming: without looking at the implementation, you have no insurance that it will actually check whether preconditions hold or not.

Now, if we shift the perspective to SFML implementation, some assertions might be useful in some places such as checking private functions' contracts or communication with third-party libraries. And it might be interesting to keep that in mind, especially when working on V3.

[Nexus]

Those are good points indeed, binary1248. In your earlier post, you focused on the technical and not library maintainer side, and I responded because I disagreed with statements like "assertions are not for library users" or "with debuggers/tools you can achieve the same"

What I don't exactly understand is Hiura's argument: the assertions are an implementation detail, an additional safety check -- they're neither something that must be documented nor something the user can rely on. They merely catch some common mistakes in the usage.

But I agree that SFML has to be consistent here, hence my earlier "maybe we should add them in other places, too"... But I see now that I underestimated this a bit -- and in cases like this particular one and possibly more, we don't even add safety (as stated in my 2nd post).

[Hiura]

What I don't exactly understand is Hiura's argument: the assertions are an implementation detail, an additional safety check -- they're neither something that must be documented nor something the user can rely on. They merely catch some common mistakes in the usage.

My point was more about defensive programming strategy in itself: since they are not documented the user cannot rely on them (and it would be a poor decision to rely on them if they were documented -- okay, debatable opinion but not essential here) and thus has no reasons to believe there are any assertions. Hence, he doesn't have any actual benefits in using debug binaries for SFML in the (debug) configuration for his application. (*) Therefore we don't catch more common mistakes.

(*) the fact that there is a bug in SFML and using debug binaries to catch it is a different story; same goes for the "feature" in MS compilers forcing users to not mix debug/release binaries -- but I guess we all agree on that.

[shadowmouse]

I'm going to leave this feature now, as has been said, this particular case isn't particularly beneficial, my point was that I thought it was pointless to purposefully leave easily solvable chances for the function to take invalid inputs, after all, the documentation makes it sound worse that it actually is. Oh, and binary, sorry for re-escalating, I probably over-reacted to what I thought was a "get in your place, programming noob" style post. But as I said, I'll leave this before anything else I say causes a full-scale war of the programming gods.

[Tank]

I myself use assertions all over my code and find them great, and yet I would never consider adding them as part of the public API of any library I write.

But why? Why are you using them yourself (out of reasons, I suppose, which bring advantages?), but not in SFML? That's something I really don't get. I mean, assertions *do a thing*, it's not like saying "I like chocolate ice cream, you like vanilla.".

Now I ask again: Would assertions benefit those who don't bother posting on the forum when they don't have a problem with SFML?

Assertions only help when you *have* a problem.

If assertions aren't targeted at them, and more experienced users don't make the kind of mistakes that are caught by assertions that often, then who are they targeted at?

They are targeted at everyone. Assertions increase code safety, so if things go wrong, anyone can benefit from it. Beginners and experts make mistakes.

Also, if we went ahead and added an assertion here, we would have to add them everywhere else as well to stay consistent.

Sure thing. It would be part of a consistent error reporting strategy.

I'm just estimating at this point, but this is at least a few hundred lines of new code, maybe even up to or over 1000. This adds to the total maintenance cost of the library, considering that the assertions would have to be adjusted, possibly at multiple remote locations of the library any time we decide to change any pre-/post-conditions. Is this extra maintenance cost worth that little peace of mind that assertions might give us?

Yep. For me code safety is very high up on my list of important requirements for code in general. Even if it costs effort, I'm happy to have it. It has already saved my life multiple times, which made me value it a lot more.

Like I said, there are tools that are more effective at catching programmer errors than manually maintained asserts.

Yes, they are good at detecting programming errors, but they fail miserably for logic errors. Like I said before, those are the errors that are really tough to debug. If one single assert() triggers due to malicious user input, then this already paid off.

Having a tool that goes hand in hand with the toolchain you are using is much more powerful than anything a library writer could ever add to the library, that is of course if people use those tools in the first place.

I think it ADDS to safety, not replaces it. You can use the tools on top.

Lastly, we mustn't forget that like everybody has already stated, assertions only have an effect in debug configurations.

This is indeed a problem I can see as well. The best we could do is spending a tutorial or something that explains this.

Also, think of the people who make use of the SFML packages on Linux systems, they won't benefit from the assertions at all since the packages rarely if ever come with a debugging configuration.

At least Debian-based systems all provide *-dbg packages. But in general it's an issue, yes.

All I am asking for at this point is tangible proof that the change we are going to make is actually going to be effective (and more effective than its alternatives) by benefiting more than 1% of the typical SFML user base. Everything posted up to now has failed to meet that requirement, talking around the actual point. Provide proof and I will happily concede.

I'm not sure what kind of proof you are looking for. I don't think I have to prove that assertions increase code safety. And increased code safety benefits everyone. I tend to declare this as a fact. The problem of using a debug build is a big one, and the effort of adding all assertions initially is huge. But other than that, I'm not sure what I should prove.

I'd say that this is at least not a high priority at all, and as error reporting in SFML is going to overhauled in the future at some time anyway, this is for sure a topic that we can bring up again.

@shadowmouse:
"We are not struggling, we are discussing!" -- <insert names of any parents here>

[eXpl0it3r]

So what's the verdict here?

For now, I'm going to close the related PR.