if(arr) now a warning

Jonathan M Davis via Digitalmars-d digitalmars-d at puremagic.com
Sat May 2 02:20:49 PDT 2015


On Friday, 1 May 2015 at 09:54:43 UTC, Walter Bright wrote:
> This really blows. And things like that isnan => isNaN really 
> has GOT TO STOP. Just today I found it broke some older piece 
> of code I had that's perfectly correct.

I don't know anything about that one, so if that's a recent one,
I missed it, but we've been rejecting name changes like that for
a while now. I think that it's been a couple of years since the
last time I saw one merged in. There might be an exception or two
that I don't know about, but we've pretty much stopped doing
precisely because it's an aesthetic thing but does not actually
catch bugs or anything like that.

That being said, something like disallowing if(arr) is completely
different. It's like disallowing implicit fallthrough in switch
statements. The change may or may not be worth making, but it's
_not_ for aesthetics. It's in order to catch and prevent bugs.
And for whatever reason, you never seem to understand that not
all breaking changes are equal. Some - particularly those that
catch or prevent bugs - are frequently welcomed, and it annoys
people quite a bit when such changes are refused. I know that Don
loves to complain about that. Changes like getting rid of
implicit fallthrough in switch statements are desirable, because
it catches and prevents bugs. And disallowing if(arr) is in the
same boat. Maybe it's not used incorrectly on a frequent enough
basis to be worth the code breakage. That's an open question. But
the change is targeted at bugs, not aesthetics, like isnan =>
isNAN is. So, the two cases are completely different.

Personally, I'd advise that no one ever use if(arr) and that they
should always do if(arr !is null) or if(!arr.empty) so that it's
clear what they intended and so that they don't accidentally
check for null when they intended to check for empty. So, I think
that we would very much be better off with if(arr) being illegal.
I don't think that it's the end of the world if we keep it, just
like it wouldn't have been the end of the world if we'd keep
implicit fallthrough for switch statements, but I do think that
it's worth making the change, because it catches and prevents
bugs.

The only problem I see with making the change has been a couple
of very vocal folks who have some code where they used if(arr)
correctly, and they don't want to change their code, which I
understand, and maybe that's enough that we won't make the change
(since it's Andrei and Cybershadow who are complaining), but most
of us consider if(arr) (or any case where an array is implicitly
converted to bool) too error-prone to use and would _want_ it
found and removed from our code bases. And it _is_ the sort of
thing that many newbies will screw up.

Regardless, whether we make the change with if(arr) or not, I
think that main point here is that not all breaking changes are
equal. Some are for aesthetic purposes (e.g. isnan => isNaN) and
don't really help much beyond having the library be more
consistent and easier to learn, whereas others actually find and
prevent bugs or allow us to implement some new idiom that we
think is important. I really don't think that the simple fact
that a change will break existing code in and of itself says
whether we should make the change. It makes it so that the change
has to provide _much_ greater value than would be the case for a
non-breaking change - the bar on such changes needs to be higher
- but sometimes, they're worth it, and it seems like you tend to
reject any breaking change even if everyone else involved thinks
that it's worth it.

We _should_ be rejecting aesthetic changes at this point, but
that doesn't mean that we should reject _all_ breaking changes.
Given your attitude on stuff like this, I'm honestly very
surprised that you ever let implicit fallthrough on switch
statements be made illegal.

- Jonathan M Davis


More information about the Digitalmars-d mailing list