I don’t find “appeal to Authority” a very convincing argument, there’s no point to debate the merits, if ya’ll go “The core devs like this style better and this is what we’re gonna do” fair enough for me, that’s your prerogative, I won’t stand in the way.
However if we do debate merits, the only additional context I see if (thing == nullptr) providing over if(!thing) is that thing can definitely be compared to a pointer type in the if (thing == nullptr) case, however nothing can be said about the type of thing itself.
While it hard to deny it adds information, are if statements really the correct place to convey this additional information? also how important is having this information at the if call site?
I don’t feel it’s very important, but I’m open hear different opinions on this
I’m with @julianeisel on this one. I may use explicit != nullptr on occasion, but in general, I try to avoid it. The danger of accidental assignment instead of comparison is real (as evidenced by the commit Julian linked to) and may lead to hard to trace bugs.
I find it an important one, but indeed not something that’s enough to sway the discussion one way or the other. If there was no point in discussing the merits, I wouldn’t have started this discussion in the first place.
About if (thing == nullptr) being bug-prone: With the danger of sounding old, I’ve been programming C++ for approx 25 years now, and I think I can count how many times I made that particular mistake on one hand. Yes, the mistake happens, but so do countless others. When it comes to avoiding bugs, I think clarity of code by reduction of complexity and by having a good architectural design is far more important than this entire discussion.
Good question, made me think. Now that I have thought about it, I think it is indeed a nice place to have such information conveyed. After all, it’s the branches in the code that add the complexity. All the nested ifs and fors and whiles add up. It’s exactly at that spot of increased complexity where you want to have as much clarity, and thus as much local understandability. Of course this is again subjective, as some people will say if (thing) is more clear than if (thing != nullptr).
The more I look at the issue, the more I feel unconvinced by either “always implicit” or “always explicit” arguments.
if (thing): compact, and positive. This matches what most people think, “if the thing is there”. Could mean multiple things, though; the interpretation depends on the definition of thing. Does it have a convert-to-bool function? If it’s there, is that function NULL-safe? If not, this expression could cause nasty issues. Makes no distinction between “is there but false-like” or “is not there”.
if (!thing): compact, but with same issues as above.
if (thing != nullptr): explicit, but has a hidden double negation. To my mind this reads as “if the thing is not non-existant”, and I don’t quite like such double negations.
if (thing == nullptr): explicit, and without double negation. No questions about what this means.
Personally I would turn the above into a set of heuristics like this:
If you want to check for NULL-ness, for example to return a default value when a parameter wasn’t passed, or to perform an allocation, use if (thing == nullptr). Using if (!thing) would cause double allocation if thing was already allocated but empty.
If you want to check for “has usable content” (i.e. non-empty string, non-empty list, allocated object, etc.), use if (thing). Using the explicit approach would require two checks here, f.e. if (thing != nullptr && !thing.empty()).
I’ve been programming for 40 years and don’t think I have ever found if(thing) to be unclear.
Does anyone really look at if(dis && dis.dat && dis.dat.splah) and think it would more clear as if((dis != nullptr) && (dis.dat != nullptr) && (dis.dat.splah != nullptr))
If we really have a preference for the IF statement to give hints about the enclosed variable types, then we should also prefer if(boolthing == true) and if(chararray[0] == \0) or perhaps ensure that all variable names contain their type like if(thing_of_type_int == 1)
I don’t must anything. I just want to get noses pointing into the same direction.
@Harleya You’re in favour of implicit checks, that’s fine, you made that point before already. Coming up with ridiculous extrapolations doesn’t help in actually discussing the topic at hand.
It’s clear that most people in this discussion prefer the implict checks; some like them always and some like them in certain cases. If we can’t decide, shall we just stick to the C++ Core Guidelines as @julianeisel described in Code Style: pointer comparisons ?
Jeesh man. I’ve been trying to summarize, to present a balanced view of pros and cons of either approach, and trying to find some consensus. What I meant to say is that repeating the same thing that was said months ago doesn’t really add anything new.
I will always vote for implicit checks just like for booleans, though I already see so much explicit checks in the Blender source.
I remember in the Uni - first year - if you coded in C some condition like… if (boolean == True)
it was bad, directly, they were really serious, even though it does the same as if (boolean)
because they wanted you to avoid redundant code,
and the truth is you read both the same but one is more direct and clean, specially for multiple checks…
I could undertand that you want to do explicit checks for pointers if you were using implicit checks for every boolean, just to not mix both and cause confusion, but I see so much redundant explicit checks of non-pointers so I guess that’s not the point? (correct me if I’m wrong).
This is exactly why I wanted the discussion: to maybe choose between two approaches that are both currently used in Blender. In light of the discussions so far, it seems like a hard choice between always one or always the other isn’t desirable.
I tried to summarize earlier arguments, and to make some new proposals based on the insight people offered. Nobody is responding to those, though, and we keep going in circles. Because of that, I’ll close this thread. If you want to talk about the subject, DM me here or on Blender Chat.
TL;DR:
If you’re here to just read the final result of this discussion and want to know what’s the accepted thing to do: just follow the C++ Core Guidelines as Julian describes in this comment and you’ll probably be fine. It’s more important that code is clear & understandable than to follow a specific strict guideline.