Hacker News Reader

Comment by qntm | Hacker News Reader

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+");
    }

View on HN

I 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.

I'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?

Certainly.

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.)

Thanks - 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).

For 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 [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 */
   }

That 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.

Yeah, 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.

Reply on news.ycombinator.com