4 ms·
In addition to everything else, please don't use a negative condition in an if/else statement. if(NULL != fname) { g = fopen(fname, "w+"); } el
by qntm 15y ago
In addition to everything else, please don't use a negative condition in an if/else statement.
if(NULL != fname) {
g = fopen(fname, "w+");
} else {
g = tmpfile();
}
Better:
if(NULL == fname) {
g = tmpfile();
} else {
g = fopen(fname, "w+");
}
- chalst 15y agoI don't agree here. != is a primitive, not a negative condition in the usual sense. Putting things this way around is natural, where the usual action appears first and the exceptional action appears later.
- arethuza 15y agoI'm not particularly disagreeing with the point you are making - but would it really be asking to much for people to provide explanations of why one form is preferred over another?
- qntm 15y agoCertainly. The else block in this case is executed when "NULL != fname" is false. That's an unnecessary double negative and takes a nonzero amount of mental effort to unscramble. The example I gave reverses this. This would also be acceptable: if(fname) { g = fopen(fname, "w+"); } else { g = tmpfile(); } (I also don't regard either statement as an "exceptional condition", they both seem equally likely to me.)
- arethuza 15y agoThanks - I've started getting a bit grumpy about coding rules that come without any explanation of why you should do something (simply saying something is "best practise" often being the worst "explanation" of all).
- VBprogrammer 15y agoFor what it's worth I couldn't disagree more. I use the guard pattern very often (as described here http://video.ias.edu/webfm_send/1207 http://video.ias.edu/webfm_send/1207 [Video]). Rather than writing: if (argc == 3) { if (fname) { /* Do stuff */ } else { printf ("File name cannot be null"); } } else { printf ("Wrong number of arguments"); } I would write: if (argc != 3) { printf ("Wrong number of arguments"); } else if (!fname) { printf ("File name cannot be null"); } else { /* Do stuff */ }
- qntm 15y agoThat has merit. Personally I usually find myself doing something more like this: if (argc != 3) { printf ("Wrong number of arguments"); // throw an exception or return NULL or something } if (!fname) { printf ("File name cannot be null"); // throw an exception or return NULL or something } /* Do stuff */ The important thing, to my mind, is the absence of an else block.
- VBprogrammer 15y agoYeah, that is a pattern I occasionally use but I prefer what I have written above as the control flow is more explicit. A missing return (or even one which is quite deliberately omitted) is much harder to spot.