Is an If branch that does nothing a code smell or good practice?

Question

I've responded to threads here (or at least commented) with answers containing code like this, but I'm wondering if it's good or bad form to write a series of if branches with one (or more) of the branches doing nothing in them, generally to eliminate checking for null in every branch.

An example (C# code):

if (str == null) { /* Do nothing */ }
else if (str == "SomeSpecialValue")
{
    // ...
}
else if (str.Length > 1)
{
    // ...
}

instead of:

if (str != null && str == "SomeSpecialValue")
{
    // ...
}
else if (str != null && str.Length > 1)
{
    // ...
}

And, of course, this is just an example, as I tend to use these with larger and more complex classes. And in most of these cases, a null value would indicate to do nothing.

For me, this reduces the complication of my code and makes sense when I see it. So, is this good or bad form (a code smell, even)?

Answer 1

I prefer doing it like this-

if (str != null) 
{
 if (str == "[NULL]")
 {
    // ...
 }
 else if (str.Length > 1)
 {
    // ...
 }
}  

I think you can always "reword" an if with an empty body into it's negation with a body, and that it looks better and makes more sense.

Answer 2

I would normally put a return or something like that in the first if:

void Foo()
{
  if (str == null) { return; }
  if (str == "SomeSpecialValue")
  {
      // ...
  }
  else if (str.Length > 1)
  {
      // ...
  }
}

If you can't do this, because the function does something else after the if/else, I'd say it's time to refactor, and split the if/else part out into a separate function, from which you can return early.