Hacker News Reader

Comment by qntm | Hacker News Reader

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

View on HN

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