C # - write blocks
This is a general question about the building blocks of a method. Does anyone have an opinion on what is the best way to design methods given the two 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;
}
I prefer method 1, the "early exit" approach. In my opinion, this is more understandable. And I really try to avoid a lot of nested if statements.
Also, you cannot return 'null' in a void method :)
Personal preferences.
I guess I would do it more like this:
if(A && B && C) {
// Do Work...
return;
}
The first method is controversial - personal preference to have multiple conditions or one OR.
However, the second method is bad, bad, bad. Multiple levels of indentation in a single method just requires bugs - maintainability and readability go downhill. It is well known and widely written and documented .
A method that starts with three if statements before any useful work can have a bad code smell.
If your method starts with a line of if statements, pause for a moment, repeat the factor, and then continue.
Perhaps a statement switch
would be a better candidate than a string of if statements.
You are trading tabs for refund operators. You make a call.
Personally, I like # 2 for C ++ where you are dealing with autoplaying objects (like CComPtr, AutoPtr, etc.), but C # is unnecessary. I would like to use the first example for C #, since you have garbage collection and you can worry less about cleaning up everything in early exits.
I'll go out to the limb and say that this is probably a code smell. I don't think one should dogmatically adhere to the old "only one way out" coding philosophy, but having multiple exit points in a method can be as much a maintenance nightmare as deeply nested IF statements.
Refactoring lights should be scattered all over the place when looking at any of them.
I try to always and always look for something that looks pretty easy:
private void Method1()
{
if (!A ||
!B ||
!C)
{
return;
}
else if(D)
{
return y;
}
// DO WORK...
return X;
}
but as others have mentioned this is entirely personal prefix, unless your workplace specifies one standard above the others.
The precondition logic can be scattered separately:
private Boolean CanContinue()
{
return A && B && C;
}
private void Method1()
{
if (CanContinue())
{
// Do work...
}
}
This has several advantages:
- Precondition logic is a potential candidate for unit testing.
- Logic can be reused.
- When the logic is complex, I think it is much easier to understand and maintain if it is encapsulated in its own method.
What about:
if(A && B && C)
{
//Do work
}
return;
or
if(!A || !B || !C)
{
return;
}
//Do work
Well, firstly, the void method cannot return a value, but on condition that you return an int ?.
private int? method3 ()
{
int? retVal;
if (A && B && C)
{
// do work
retVal = x;
}
return retVal;
}