C# Blank else to escape if bad practice?

Question

I mean would it be bad practice if I want an if statement to have an else, but there is a nested if statement "in my way" so I use a blank else statement such as "else ;" or "else { }" to escape it?

ex:

if (lsvAddons.SelectedItems.Count > 0)
    foreach (ListViewItem A in lsvAddons.SelectedItems)
        if (Addons[A.Index] != null) Addons[A.Index].DoHelp();
        else { }
else Console.WriteLine(_GenericHelpString);

vs:

if (lsvAddons.SelectedItems.Count > 0)
{
    foreach (ListViewItem A in lsvAddons.SelectedItems)
        if (Addons[A.Index] != null) Addons[A.Index].DoHelp();
}
else Console.WriteLine(_GenericHelpString);

Or even:

if (lsvAddons.SelectedItems.Count > 0)
{
    foreach (ListViewItem A in lsvAddons.SelectedItems)
    {
        if (Addons[A.Index] != null) 
        { 
            Addons[A.Index].DoHelp();
        }
    }
}
else
{
    Console.WriteLine(_GenericHelpString)
}

Answer 1

Use the third formatting.

Answer 2

If you're asking which is nicer, cleaner code, then the last one!

Answer 3

An empty code block is bad practice.

else { }

Answer 4

Use curly brackets ({ and }) to specify to which if your else belongs. By default, it belongs to the if closest to it in the same scope (the curly brackets). Using brackets also improves readability for you in the future and other developers which might read it.

So, use your second formatting to solve you problem.

if (lsvAddons.SelectedItems.Count > 0)
{
    foreach (ListViewItem A in lsvAddons.SelectedItems)
        if (Addons[A.Index] != null) Addons[A.Index].DoHelp();
}
else Console.WriteLine(_GenericHelpString);

However, I would prefer the third example you gave, for readability.

Answer 5

Braces are your friend.

You'll be ahead of the curve if you resist the urge to omit braces. The third form is completely unambiguous, easier to read, and will help you avoid inadvertant errors. I don't know about you, but I can understand the behavior of the third form right away .. the first two forms require more thinking and mental parsing to get right.

Many developers try to get rid of braces and use the implicit form of if statements. Unfortunately, what do you do when you run across something like:

if( someCondition... )
    DoSomething();
    DoAnotherThing();

Did the developer simply indent incorrectly? Or was the intent to have both methods part of the same condition? It can be hard to tell after the fact ... and is a leading source of defects. Here's an even worse example:

if( someCondition );
    DoSomething();
    DoAnotherThing();

Do you notice the subtle mistake here?

You're much better off in the long run if you make your code unambigous and less prones to these kinds of problems.

Often developers argue to omit braces so that their code is shorter and easier to understand. Now, there's certainly merit to avoiding long methods (some even argue that methods should always be 1-screen long) ... but I don't think the risk introduced by this kind of brevity is generally worth the reward. Keep in mind, there are ways to restructure methods to avoid excessive nesting - and they often gain you more in readability than omitting braces do.

My personal mantra is:

1. Make it correct.
2. Make it clear.
3. Make it concise.
4. Make it fast ... in that order.

Answer 6

Your second version is "best practice". ReSharper will complain about the empty code block, and it's generally better to use fewer keywords than more. Yes, version two takes up more whitespace, but that's not necessarily a bad thing, and it has fewer non-whitespace chars.

Answer 7

Does the empty else make your code more readable? No, but the braces do.

Answer 8

It took me a while to understand what everything belongs to in the first example, so I'd say it's bad practice.

Personally I

1. Never ever nest braceless control structures because it can be very hard to read.
2. Use braces on either both the if and else, or neither.

So I'd format it as follows:

if (lsvAddons.SelectedItems.Count > 0)
{
    foreach (ListViewItem A in lsvAddons.SelectedItems)
    {
        if (Addons[A.Index] != null) 
            Addons[A.Index].DoHelp();
    }
}
else
{
    Console.WriteLine(_GenericHelpString)
}

Answer 9

Readable means => best practice. Using bracers always saves the day, don't forget that.

Answer 10

Excessive braces are unnecessary and annoying.

I don’t mind braces, and I think if you have several levels of nesting, you should use them even if there is only one statement. But you shouldn’t religiously use them all the time.

For something very simple like

if (some_condition)
    Console.WriteLine("some info");

using braces would be completely over the top, and would make it much harder to see the bigger picture of the surrounding code (because you can fit less of it on the screen).