Something simple I wish I learned 17 years ago | Hacker News Reader

Something simple I wish I learned 17 years ago

41 points3pt1415964 comments

I'm sure many of you guys and gals know this. And it might be just one of those things that I have somehow missed during all my time coding QBASIC, VBA, C++, C, Ruby, Python, etc...

Most of the time when you are writing an if statement, you are comparing a mutable variable to something constant (i.e. variable to string, variable to integer, variable to symbol), so you can avoid the assignment-instead-of-comparison bug by writing constant == variable instead of variable == constant. This way if you forget one of the equals signs you'll get a syntax error. The key with this is you have to get in the habit of doing it and do it all the time, so that you don't have a false sense of security. So instead of:

if foo.to_str == "hi everyone!\n"

  puts "hello there!"

end

use

if "hi everyone!\n" == foo.to_str

  puts "hello there!"

end

View on HN

I might be in the minority, but I really don't like that style. Whenever I read:

  if (NULL == ptr) { ..

it throws me off, and it doesn't read as well as the other way around. gcc's -Wall will warn you about it (and I'm sure other interpreters/compilers will do the same). I'd rather let the compiler handle things it's good at and stick to authoring code that's easy to read.

For pointers I think it's much more readable to use boolean NOT.

  if (ptr) {
    ptr->foo();
  } else {
    die();
  }

  if (!ptr) {
    OhNoes();
  }

Also, you can extend the whole lhs-rvalue thing to for loops. Makes them look funny but you get used to it quickly, especially after it helps you avoid a few bugs.

  for (i = v.begin(); v.end() != i; ++i) {
    i->stuff();
  }

!ptr is bad.

I worked with a C programmer who changed all occurrences of "ptr != 0" to "!ptr". The program stopped working. In Windows, a null pointer is not 0. The C idiom "ptr != 0" compiles correctly, but when you try to get clever with "!ptr", you were in trouble. The expression would evaluate to false on a null pointer. (At least with the version of Visual C++ we were using back then.)

This was true years ago (and may be true still if you have to support legacy or niche compilers) but in current standard C and C++, the null pointer is guaranteed to equal zero in integer comparisons (and therefore false in boolean expressions).

http://www.research.att.com/~bs/bs_faq2.html#null

http://www.faqs.org/faqs/C-faq/faq/

> the null pointer is guaranteed to equal zero in integer comparisons (and therefore false in boolean expressions).

Note that this does not mean that the null pointer is 0.

I'll quote the relevant sentence from section 5.2 of http://www.faqs.org/faqs/C-faq/faq/: "According to the language definition, a constant 0 in a pointer context is converted into a null pointer at compile time."

Later in that section, there's an example that makes the difference between "null pointers are 0" and "constant 0s in a pointer context are converted into a null pointer". The intro to that example is "However, an argument being passed to a function is not necessarily recognizable as a pointer context, and the compiler may not be able to tell that an unadorned 0 "means" a null pointer. To generate a null pointer in a function call context, an explicit cast may be required, to force the 0 to be recognized as a pointer."

The example itself is: "execl("/bin/sh", "sh", "-c", "date", (char )0); If the (char ) cast on the last argument were omitted, the compiler would not know to pass a null pointer, and would pass an integer 0 instead."

See also 5.3 which starts with "Is the abbreviated pointer comparison "if(p)" to test for non- null pointers valid? What if the internal representation for null pointers is nonzero?"

The change reverses the logic of the expression, so I would expect the program to stop working - regardless of the value of the null pointer.

Bah. You're right. I meant ptr == 0 was changed to !ptr.

in general, i think `if (ptr==0)` is an antipattern and designing for assert(ptr!=0) yields better code.

Agreed, it feels like I am reading arabic mixed in with english. Code flows top to bottom, left to right and suddenly there is this one piece that is right to left.

I am pretty sure this is a recommendation from Code Complete, too. It's full of great tips like this. If you haven't, you should definitely check it out.

It's in a handful of canonical texts. I first encountered it in the first few pages of Debugging Applications, by John Robbins; the updated .NET version is great, but even the original Win32 edition is full of gems, just like Code Complete.

This is a habit I picked up, too. It's very rare, though; most people don't do this, and think my code just looks weird. :) But I'd rather see this, than have obscure bugs from accidental assignments.

Yep. Had a "lead" developer tell me, "Your conditions are backwards." -_-

Well, whether it's good or not is debatable, but they are backwards.

Equivalence relations are non-directed. There is no formal sense in which one or the other can be said to be backwards.

It's only personal preference that leads you to say that, and in this case, for the reasons cited above, it is deeply suboptimal personal preference.

This is a case of Diax's Rake at work.

Still, conventions matter. If you're reading C code, and you see "for (i=0; i<N; i++) { ...", you know what it means, but "for (i=0; N>i; i++) { ..." probably makes you double-take, even though it's equivalent.

If your project's code standard can include testing for (constant == variable), do it, but the impact on readability shouldn't be ignored. (I agree that it's ultimately a language problem.)

Actually, no. Many languages implement == as a operation on the object itself, i.e. invokes a method named == on the object on the left side of the statement. If the things you are comparing are not the same class, then == is very much a directed relation. I agree that logically it might not be, but in many implementations, it actually is.

When it's that kind of comparison, that's an argument for putting the constant on the left. Then you'll know you're getting the kind of equality method defined for the constant (which is more typically what you want).

Nice one, never occurred to me even though I sometimes would write my assignments this way. I guess the problem is it doesn't generalise to the case where you are comparing two variables.

Fortunately not a problem with Python (the pedantism works!). I still think I prefer Pascal's := for explicit assignment though. Wish more languages would adopt it.

Yeah, MySQL does that as well. That is one of my big problems. I program 70% of the time in MySQL where "=" is used for comparison, so it is hard to switch to "==" when I occasionally need to.

I've seen this in an old book (Code Complete?) a few aeons ago, and I can't say I like it, but I use it a lot.

In Java/C# this can throw a null pointer exception:

myString.equals("another string");

however this will never throw anything:

"another string".equals(myString); //false if myString is null, this is what you mean in most cases anyway

in c# you wouldn't use the .Equals() method. You just do myString == someOtherString. Also it has a nice static method on the string class: if( string.IsNullOrEmpty(mystring) ){}

I use that like crazy. I'm not sure if Java has something similar now. Years ago when I used java, I remember being constantly frustrated with having to write a helper method in every project to do { return myString==null || myString.equals("");}

I did find "something".equals(myString) helpful though.

I think it hurts readability. But sometimes it is nice to skip a null check. i.e. instead of:

if (a != null && a.equals(b))

You can often do:

if (b.equals(a))

Those are not strictly equivalent, in that b.equals(a) could evaluate to true when both are null which would fail in the first form you gave.

I'm sort of reaching here, because I don't know off-hand which language you are using, but I would assume that NULL.equals() would throw an exception. That exception would not be thrown/need to be caught in the first example.

Maybe a good tip for beginners who confuse = and ==, but I don't recall ever making this error, and I've written a ton of code.

I agree. I saw this tip a long time ago and tried it for a bit, but realized that I don't need it because I never put a "=" in place of a "==".

Then you weren't writing that code in C.

I've written a lot of code in C and never run into this error. And I never put the constant on the left.

I have also written a ton of code, and the times I have made this error have been sufficiently annoying that I've forced myself to reverse the comparisons.

I picked this up a few years ago:

http://c2.com/cgi/wiki?CompareConstantsFromTheLeft

I've mostly stopped doing it though, because now I usually work in languages that won't let you put an assignment in an if statement, and nobody else really does it.

yeah, i only put the constant on the left in languages that let you do assignments in the if, like C++.

For me one of the big things that I need to constantly remind myself of is to read what is on the screen, instead of what is in my cache. While this trick may help with testing for equality I'll often mix up the bit-wise and logic operators as well, not so mention the misplaced semi-colon that is so hard to see. Reading what is actually written has saved me numerous errors and is invaluable for code reviews and the like.

That's a nice workaround, but to me that smells like something wrong with the language itself. I can't think of a good reason to allow assignment in an if statement; other than maybe save a few characters and introduce a lot of confusion / bugs in the process. If you need to make an assignment then do it separately, don't hide it in an if statement (imho).

It helps a lot with idiomatic C error handling. You'll often have a function that does something, returns 0 on success, and an error code on error. It's nice to be able to do the assignment in the if statement there.

There's no good reason for it in any language built with exceptions from the ground up, though.

Or with languages that have a better mechanism for returning multiple values. (Passing in pointers to optional results? Ick.)

There's in idiom in Lua to have functions return either <true, result> or <false, error message> (as two arguments, not a tuple) - consequently, it's common to say e.g. "status, f = assert(open_resource(arg, arg2))"; if it fails, the assert will fail, and open_resource will provide the error message.

In statically typed languages, returning a union type (e.g. Error | Pass of Result) also works well. Sometimes exceptions are simpler, sometimes not.

Good compilers will warn you.

Extend this to functions as well.

I had a bug like

if (foo(b == A))

instead of

if(foo(b) == A)

Good compilers will warn you.

Even most bad compilers warn about this now.

Even ruby warns you

puts 'argh' if a = 'b' warning: found = in conditional, should be ==

Yes, but these warnings can generate false positives if you actually want to do assignment in a test condition. And before everyone tells me that's bad coding style, it is at least a common idiom in certain kinds situations like this very common example from PHP (whose compiler does not warn for assignment at all):

    <?php
    if (false !== ($pos = strpos($haystack, $needle))) {
        doSomethingWith($haystack, $pos);
    }
    ?>

I find the constant first notation to be very friendly, btw. It's not as natural when translated into English normally, but in terms of logic, it makes sense to think of the first part of the test condition as its own little predicate:

    // Generic language
    bool matchesMyConstantValue(val) {
        return MY_CONSTANT_VALUE == val;
    }

Agreed and a more general case of that is something like:

  if ($result = $o->someMethod()) {
	// Do Something with $result, the method returned a value that cast to true
}

I would argue that this is pretty unreadable. Why do obfuscate your code to save one LOC?

It's common in C where functions return an error code instead of a value that you would use again. Then the conditional just checks that you're OK to go ahead, and you don't need to declare another variable to store the error code. (If a language has exceptions, then this isn't as useful.)

I write C full time for the day job and it normally goes something like this:

if ( !method_returns_rc( args ) ) { //error handle }

This isn't great, as I believe its not good to put methods with side effects in if statements (may be wrong here, or maybe its just me), but generally thats how its done (not to say that there aren't freaking GOTOs in our code... so take that with a grain of salt.)

One man's obfuscation is another man's expressiveness.

When gcc warns you, it tells you to wrap it in parens if you really mean it: "warning: suggest parentheses around assignment used as truth value". Your PHP example wouldn't raise a warning from gcc, because it already is.

I find assignment-in-conditional more useful in loops than if's, since you can't just plonk the assignment on the previous line.

I am not sure where I read this, but AFAIR it's from "The practice of programming" from Pike and Kernighan (1). The suggestion circumventing this was to not use a space before the assignment, i.e.:

"a= b;" instead of "a = b;"

I have been doing this constantly in all C-style languages and derivatives and never had any problems using this. This is probably less invasive that your initial suggestion and preserves readability/the usual style.

[1]: To anybody who knows this, too: I am really, really, really unsure where I read that and it's been about 15 long years since I have been doing it, so any correction is much appreciated.

I dunno, I find that I just don't make these errors these days. I mean, I obviously did when I started out, but it didn't take all that long to internalize the fact that I need to do == to compare and not =.

Joel posted about it once.

(Found the link:) http://www.joelonsoftware.com/articles/fog0000000073.html

"Occasionally, you will see a C programmer write something like if (0==strlen(x)), putting the constant on the left hand side of the == . This is a really good sign. It means that they were stung once too many times by confusing = and == and have forced themselves to learn a new habit to avoid that trap."

Its not an all the time thing, though. I often use assignment in a condition statement, in Ruby:

  if value = obj.slow_method
    puts value
  end

I know I had several bugs from the assignment back in my days of coding C/Java in college, and early in my career as a PHP developer, but I can't recall having an issue with it since programming in Ruby. Perhaps the language flow or best-practices manage to avoid the cases when that bug is possible.

If you make it a habit to always reverse comparisons, then the assignments stick out that much more, and remind you that it is, in fact, intentional.

I probably wouldn't pick up this habit simply because

1) You shouldn't be able to make an assignment (by itself) in an if and not get a compiler error in most modern languages.

2) It goes against the logical flow. You are checking if a variable is equivalent to a constant, not if the constant is equivalent to your variable. If your code is read by someone else who is unfamiliar with this practice then they may get confused by the illogical flow of everything.

About 2): So does "string".equals(variable), which is also 'backwards' but safer.

I'd always assumed this was a common teaching. I saw it repeatedly in my undergrad CS days, and even saw it in some AP high school classes.

I once read a tip that said:

"Read '=' as 'equals'. Read '==' as 'is equal to'"

I made this error much less after picking up that habit.

For efficiency reasons, try to stop reading source code in your mind. It's best to just feel it. Reading is is akin to mouthing words in a novel; the lip movement is distracting and slows you down.

FWIW, I read = as "set" (the verb "to set") or "assign" or "let".

it's a good trick but letting the compiler warn you is better (and of course always compiling warning-clean so you'll notice it). There's plenty of times where both sides are non-const and this trick won't save you when that happens.

I guess i read it in Code Complete. Yes, it looks ugly; yes, it avoids many bugs.

This is in my project coding standards document and one I actually like :)

It's so nice I do not have to worry about this when writing in python.

"Named must your fear be, before banish it you can."

What's with all the Yoda language here lately, is it a Reddit thing that's spreading?

It's a demonstration of how backwards-sounding can sometimes be right.

you just found this? do you live under a rock?

Clever - that goes in the tool box - thanks!

Reply on news.ycombinator.com