C# - writing method blocks...

Question

Hi, this is a general question regarding method block structures. Does anyone have an opinion on what is a better way to design methods given the 2 alternatives below?

private void Method1()
{
    if (!A)
    {
        return;
    }

    if (!B)
    {
        return;
    }

    if (!C)
    {
        return;
    }

    // DO WORK.......

    return;
}

private void Method2()
{
    if (A)
    {
        if (B)
        {
            if (C)
            {
                // DO WORK.......

                return;
            }
        }
    }

    return;
}

Answer 1

I prefer method 1, the "early exit" approach. In my opinion, it's more clear. And I really try to avoid lots of nested 'if' statements.

Also, you can't return 'null' in a void method :)

Answer 2

Personal Preference.

I think I would do it more like this:

if(A && B && C) {
   // Do Work...
   return;
}

Answer 3

The first method is debatable - personal preference to have multiple conditions or a single OR.

However the second method is bad, bad, bad. Multiple levels of indentation in a single method is just asking for bugs - maintainability and readability go way down. This is well-known, widely written about and documented.

Answer 4

You're trading tabs for return statements. You make the call.

Personally I like #2 for C++ where you're dealing with auto-releasing objects (e.g. CComPtr, AutoPtr, etc) but there is no need for that in C#. I would tend to use the first example for C# since you have garbage collection and you can worry less about making sure you cleanup everything on early exits.

Answer 5

Well, first, a void method can't return a value, but assuming you're returning an int?.

private int? method3 ()
{
  int? retVal;
  if (A && B && C)
  {
    // do work
    retVal = x;
  }
  return retVal;
}

Answer 6

What about:

if(A && B && C)
{
     //Do work
}

return;

or

if(!A || !B || !C)
{
     return;
}

    //Do work

Answer 7

I will step out on a limb and say that either is probably a code smell. I don't think one should dogmatically adhere to the old "only one exit" philosophy of coding, but lots of exit points in a method can be just as much of a maintenance nightmare as deeply nested IF statements.

Refactoring lights should be going off all over the place when looking at either of those.

Answer 8

Put me down for #1.

Answer 9

I tend to try and always go with something that looks fairly easy to read:

private void Method1()
{
    if (!A ||
        !B ||
        !C)
    {
        return;
    }
    else if(D)
    {
        return y;
    }

    // DO WORK...

    return X;
}

but as others have mentioned it's entirely personal pref unless your workplace specifies one standard above others.

Answer 10

Pre-condition logic can be factored out into a separate method:

private Boolean CanContinue()
{
  return A && B && C;
}

private void Method1()
{
  if (CanContinue())
  {
    // Do work...
  }
}

This has several advantages:

• The pre-condition logic is a potential candidate for unit testing.
• The logic can be re-used.
• When the logic is complex, I think it's much easier to understand and maintain if it's encapsulated in its own method.

Answer 11

A method which starts with three if statements before any useful work can be done has a distinctively foul code smell.

If your method begins with a string of if statements, stop for a moment, re-factor, then proceed.

Perhaps a switch statement would be a better candidate than a string of if statements.