Should an Else statement be used just for a comment?

Question

I came across the following in some sample code:

if (urlStr)
{
    NSImage *iconImage = [[NSWorkspace sharedWorkspace] iconForFile:urlStr];
    node.nodeIcon = iconImage;
}
else
{
    // it's a separator, don't bother with the icon
}

Is this style of using an else statement only for a comment good practice for production or it is only useful for code used as an example?

Answer 1

Edit: my answer got some criticism in the comments, and I'm pretty sure the commentators are right and my answer is probably mostly nonsense.

Almost all of your answer is about performance, which we all agree is not an issue. The correct answer should address what is more clear, more readable, etc. – user949300

Your performance argument makes no sense, as you are compiling without optimizations. Skipping the else block is a trivial optimization for any compiler -- MartinHaTh

---

For the programmer reading it, I think it makes it quite clear. I am used to reading "if condition do stuff else do other stuff", so I can parse that pretty easily. The else block seems like a really logical spot for a comment explaining why we don't want to do anything in that condition.

But, will it make a significant impact if we have the empty block there? Is improved clarity for humans worth the cost? I think that's a language-specific (or rather, compiler/interpreter specific) question.

The only time I can think that this could have some negative impact is if it's in a bottleneck that needs to be ultra-optimized.

Even in those cases, it might make no difference. Here's an example of how I would check, for C.

sometimes it makes no difference to the compiled code

short.c

int main() {
  if(1) {
    printf("Hello World");
  }
}

long.c

int main() {
  if(1) {
    printf("Hello World");
  } else {
    // not possible
  }
}

Then I compiled both with the -S option to compile without assembling. I compared the assembly files and they were identical.

sometimes it does make a difference to the compiled code

I then updated the files and replaced if(1) with int foo = 1; if(foo) and repeated the process. (I know, the block is still not reachable - so if the compiler still treats it as unreachable I have proved nothing, but if the compiler treats it as reachable then I know I have successfully tricked the compiler and I have proven my point.) In this case, the compiled files were not identical - the compiler did not ignore the empty else block when it thought it was reachable.

short.s

  ## call to printf, etc. (i.e. the if block)...
LBB0_2:
  ## rest of code...

long.s

  ## call to printf, etc. (i.e. the if block)...
  jmp LBB0_3 ## ! there is always a jump statement
LBB0_2:
  jmp LBB0_3
LBB0_3:
  ## rest of code...

So in this almost reachable version, when the empty else block is included, there is an extra jump statement at the end of the if block. So if performance is a key concern here, then adding the empty else block is not a good idea.

conclusion

Based on the language, the else block could change the compiled code and affect performance. You would have to check for whichever language you're using, and then if it does have a performance hit in your language, decide if performance is more important than readability in this section of code.

Answer 2

No, having an else clause just to make a comment is generally not a good idea, it polutes the code with unnecessary lines and slows the programmer reading it.

When a programmer reads the code his thought proces may be: Look, there's an else clause, so when the urlStr is not set, something else mig... oh, nevermind, it's just a placeholder.

By changing the code to:

bool isSeparator(string urlStr)
{
    return urlStr != null;
}

if (!isSeparator(urlStr))
{
    NSImage *iconImage = [[NSWorkspace sharedWorkspace] iconForFile:urlStr];
    node.nodeIcon = iconImage;
}

You achieve the very same thing without needing the else block and the comment.

Answer 3

No, it is very bad idea. It makes your code more complex without any benefit. If you want to explicitly state the else condition do it with a comment above the if :

// When the urlStr is false then it is a separator and no action is needed on the icon
if (urlStr)
{
    NSImage *iconImage = [[NSWorkspace sharedWorkspace] iconForFile:urlStr];
    node.nodeIcon = iconImage;
}

Answer 4

It can sometimes make sense when you want to document that you thought about there being an else-case but intentionally want to point out that it is fine to do nothing in it.

if (file.open()) {
   readStuffFrom(file);
} else {
   // file not found - but we can ignore this for now because REASONS
}

Answer 5

Though I wouldn't vote against the else case in someone else's code, personally I would prefer the following form with the rationale to avoid the additional no-op code:

if (urlStr)
{
    // it's not a separator, so we set an icon
    NSImage *iconImage = [[NSWorkspace sharedWorkspace] iconForFile:urlStr];
    node.nodeIcon = iconImage;
}

Answer 6

Counter-question that won't work well as a comment: is there a compelling reason why you can't write this?

if (urlStr)
{
    NSImage *iconImage = [[NSWorkspace sharedWorkspace] iconForFile:urlStr];
    node.nodeIcon = iconImage;
}
// else it's a separator, don't bother with the icon