Rule of precedence == over =

Question

Hello,

I am just wondering would it be better to do this:

if((fd = open(filename, O_RDWR)) == -1)
{
fprintf(stderr, "open [ %s ]\n", strerror(errno));

return 1;
}

or this

fd = open(filename, O_RDWR);
if(fd == -1)
{
    fprintf(stderr, "open [ %s ]\n", strerror(errno));

    return 1;
}

Many thanks for any suggestions,

Answer 1

Its a style thing - you're not asking a precedence (not presidence).

Many people will argue the latter example is clearer.

Answer 2

Yuck, split it up. What do you gain by mashing it all on one line? Let's compare and contrast:

Single-line:

• Advantages:
• Disadvantages: Hard to read, prone to error. (Consider your first revision.)

Multi-line:

• Advantages: Easy to read, less error-prone.
• Disadvantages:

I think it's clear. :)

---

"Sometimes putting it on one line makes more sense, for example: while ((c=getchar())!=EOF)"

That's fine, but that isn't the case here. There are times when not splitting it up makes more sense, but in general, don't.

---

"It saves more vertical space"

If one line is killing your ability to see the function, you need to 1) buy a monitor with a resolution higher than 640x480, and 2) Write smaller functions.

Really, I've never understood that argument for anything, functions should easily fit on any screen, regardless of a one-line difference.

---

"Multiple lines make it look complex"

Not really, shoving it on one line is arguably harder to read and more complex looking. Splitting things up makes it simpler to process one bit at a time, one shouldn't assume two lines makes it twice as complex.

Answer 3

This is a matter of style and is subjective. They do the same thing. I tend to prefer the later because I find it easier to read, and easier to set breakpoints/examine variables in the debugger.

Answer 4

Maybe something where the parentheses make the ordering obvious?

if((fd = open(filename, O_RDWR)) == -1)

Answer 5

Except for standard idioms -- ones that are so common that everyone immediately gets what you are trying to do -- I would avoid doing the assignment in the conditional. First, it's more difficult to read. Second, you leave yourself open (at least in weakly typed languages that interpret zero as false and non-zero as true) to creating bugs by using an errant assignment operator in the conditional check.

Answer 6

The second is better for readability's sake, but I know I do the first too often. The = operator will take precedence, especially since you have it in quotes, allowing the assigned value to be returned & compared by the == operator.

Answer 7

(-1 == __whatever__) 

to minimize typo

Answer 8

Several people have argued in favor of the second. I disagree with them. While there was (apparently) initially a minor issue with = vs. == in the first one, I'd argue that it IS a minor issue.

A much bigger issue is that it's all too common for people (especially if they're in a hurry) to skip over the error checking -- leaving out the if (whatever == -1) completely, usually on the theory that what they're working on is quick, throwaway code and checking for the error isn't really needed. This is a really bad habit; I can practically guarantee that every person reading this has seen real code that skipped error checking like this, even though it really, really should have had it.

In code like this, attempting to open the file and checking for an error in having done so should be inextricably bound together. Putting the two into the same statement reflects the proper intent. Separating the two is just plain wrong -- they should not be separated in any way at any time for any reason. This should be coded as a single operation because it should be a single operation. It should always be thought of and coded as a single operation.

The excuses for doing otherwise are, in my opinion, quite weak. The reality is that anybody who uses C needs to be able to read code that combines an assignment with a conditional test. Just for an obvious example, a loop like while ((ch=getchar()) != EOF) pretty much needs to be written as a combined assignment and test -- attempting to test for EOF separately usually leads to code that just doesn't work correctly, and if you do make it work correctly, the code is substantially more complex.

Likewise, with the problem of - vs. ==. Since I didn't see the defect to start with, I'm not sure how much separating the two would have done to avoid problems, but my immediate guess is that it probably made almost no difference at all. Compilers that will warn you when what was supposed to be a condition contains only an assignment have been around for years (e.g. gcc). In most cases, the symptoms are almost immediately obvious anyway -- in short, the fact that you made a particular typo in one part of this posting but not the other doesn't prove (or honestly even indicate) much of anything about the relative difficulty of the two.

Based on that kind of evidence, I'd apparently believe that "not" is harder to type than "immediately", since I just typed "immediately" without a problem, but had to correct "not" (twice, no less) before it came out right in the previous sentence. I'm pretty sure if we went by how often I mistype it, "the" is the single most difficult word in the English language.

Answer 9

In this example, I'll join the chorus saying the second method is better.

The tougher case is when it's in a loop, like:

while ((c=getchar())!=-1)
{
  ... do something ...
}

versus

while (true)
{
  c=getchar();
  if (c==-1)
    break;
  ... do something ...
}

In cases like that I prefer to do it on one line because then it makes clear what is controlling the loop, which I think overrides the drawbacks of the complex combination of assignment and testing.

Answer 10

The first case is very normal when you're writing an input loop, because the alternative is to have to write the input command twice -- once right before the loop, and once right at the end of the loop.

while ( (ch=getchar()) != -1){
  //do something with it
}

I think that the second way is more normal for an if statement, where you don't have the same concern.