Precedence rule == over =
I'm just wondering if it would be better to do this:
if((fd = open(filename, O_RDWR)) == -1)
{
fprintf(stderr, "open [ %s ]\n", strerror(errno));
return 1;
}
or
fd = open(filename, O_RDWR);
if(fd == -1)
{
fprintf(stderr, "open [ %s ]\n", strerror(errno));
return 1;
}
Thanks a lot for any suggestions,
a source to share
Yuk, strip him. What do you get by knocking it all down in one line? Let's compare and contrast:
One line:
- Benefits:
- Disadvantages: Difficult to read, prone to errors. (Consider your first revision.)
Multi-line:
- Benefits: Easy to read, less error prone.
- Disadvantages:
I think this is clear. :)
"Sometimes it makes sense to put it on one line, for example: while ((c=getchar())!=EOF)
"
This is good, but it isn't. There are times when not splitting it makes more sense, but in general, don't do it.
"This saves more vertical space."
If one line is killing your ability to see this feature, you need to 1) buy a monitor with a resolution higher than 640x480 and 2) record smaller features.
In fact, I've never understood this argument for anything, functions should easily fit any screen, regardless of the one-line difference.
"A few lines make it hard"
Doesn't look like dragging it one line, perhaps harder to read and more complex to look at. Separating things makes it easier to process one bit at a time, one cannot assume that two lines make it twice as difficult.
a source to share
Several people spoke in favor of the second. I disagree with them. Although there was a (apparently) minor issue with =
vs. ==
in the first, I would say it is a minor issue.
The much bigger problem is that it is too often for people (especially if they are in a hurry) to skip error checking - completely abandon if (whatever == -1)
, usually, the theory that they are working on a swift, throwing out code and checking for an error in fact business is not needed. This is a very bad habit; I can practically guarantee that every person reading this has seen real code that missed error checking like this, even though it really should have been.
In code like this, trying to open a file and check for an error while doing so should be inextricably linked together. Including two in the same expression reflects the correct intent. Separating the two is simply wrong - they should never be separated for any reason. This should be coded as a single operation, as it must be a single operation. It should always be viewed and coded as a single operation.
Excuses for others, in my opinion, are rather weak. The reality is that anyone using C should be able to read code that combines assignment with a conditional test. Just for an obvious example, a loop like this while ((ch=getchar()) != EOF)
should pretty much be written as a combined assignment, and a test - trying to test for EOF
separately usually results in the code just not working correctly, and if you do it to make it work correctly, the code is significantly more complicated. ...
Likewise, with the -
vs. ==
... Since I didn't see a defect to begin with, I'm not sure how you would separate the two to avoid problems, but I immediately think that it probably made little or no difference. The compilers that will warn you about what should have been the condition only contain the job for years (like gcc). In most cases, the symptoms are almost immediately apparent anyway - in short, the fact that you made a special typo in one part of this post and not another does not prove (or, frankly, even) anything about the relative difficulty of the two ...
Based on such evidence, I seem to think that "no" is harder to type than "right away" as I just typed "immediately" with no problem, but had to correct "no" (twice, no less) before than it came out in the previous sentence. I'm pretty sure if we shared how often I get it wrong, "this is the hardest word in the English language."
a source to share
In this example, I join the chorus in saying that the second method is better.
The harder case is when it's in a loop, for example:
while ((c=getchar())!=-1)
{
... do something ...
}
against
while (true)
{
c=getchar();
if (c==-1)
break;
... do something ...
}
In such cases, I prefer to do it on a single line because then it makes it clear that it controls the loop, which I think overrides the disadvantages of a complex combination of assignment and testing.
a source to share
Except for the standard idioms - those that are so common that everyone immediately gets what you are trying to do - I would avoid doing the assignment in a conditional expression. First, it's harder to read. Second, you leave yourself open (at least in weakly typed languages that interpret null as false and non-null as true) to create errors using the erroneous assignment operator in conditional validation.
a source to share
The first case is very common when you write an input loop, because the alternative is to have to write the input command twice - once right before the loop and once at the end of the loop.
while ( (ch=getchar()) != -1){
//do something with it
}
I think the second way is more normal for an if statement where you don't have the same concern.
a source to share