문제

I've often written this sort of function in both formats, and I was wondering if one format is preferred over another, and why.

public void SomeFunction(bool someCondition)
{
    if (someCondition)
    {
        // Do Something
    }
}

or

public void SomeFunction(bool someCondition)
{
    if (!someCondition)
        return;

    // Do Something
}

I usually code with the first one since that is the way my brain works while coding, although I think I prefer the 2nd one since it takes care of any error handling right away and I find it easier to read

도움이 되었습니까?

해결책

I prefer the second style. Get invalid cases out of the way first, either simply exiting or raising exceptions as appropriate, put a blank line in there, then add the "real" body of the method. I find it easier to read.

다른 팁

Definitely the latter. The former doesn't look bad right now, but when you get more complex code, I can't imagine anyone would think this:

public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    int retval = SUCCESS;
    if (someCondition)
    {
        if (name != null && name != "")
        {
            if (value != 0)
            {
                if (perms.allow(name)
                {
                    // Do Something
                }
                else
                {
                    reval = PERM_DENY;
                }
            }
            else
            {
                retval = BAD_VALUE;
            }
        }
        else
        {
            retval = BAD_NAME;
        }
    }
    else
    {
        retval = BAD_COND;
    }
    return retval;
}

is more readable than

public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    if (!someCondition)
        return BAD_COND;

    if (name == null || name == "")
        return BAD_NAME;

    if (value == 0)
        return BAD_VALUE;

    if (!perms.allow(name))
        return PERM_DENY;

    // Do something
    return SUCCESS;
}

I fully admit I never understood the advantage of single exit points.

It depends - In general I am not going to go out of my way to try and move a bunch of code around to break out of the function early - the compiler will generally take care of that for me. That said though, if there are some basic parameters at the top that I need and can't continue otherwise, I will breakout early. Likewise, if a condition generates a giant if block in function I will have it breakout early as a result of that as well.

That said though, if a function requires some data when it is called, I'm usually going to be throwing an exception (see example) as opposed to just returning.

public int myFunction(string parameterOne, string parameterTwo) {
  // Can't work without a value
  if (string.IsNullOrEmpty(parameterOne)) {
    throw new ArgumentNullException("parameterOne");
  } 
  if (string.IsNullOrEmpty(parameterTwo)) {
    throw new ArgumentNullException("parameterTwo");
  }

  // ...      
  // Do some work
  // ...

  return value;
}

I prefer the early return.

If you have one entry point and one exit point then you always have to track the entire code in your head all the way down to the exit point (you never know if some other piece of code bellow does something else to the result, so you have to track it up until the exist). You do that no mater which branch determines the final result. This is hard to follow.

With one entry and multiple exists, you return when you have your result and don't bother tracking it all the way down to see that nobody does anything else to it (because there won't be anything else since you returned). It's like having the method body split into more steps, which each step with the possibility to return the result or let the next step try its luck.

In C programming where you have to manually clean-up there is a lot to be said for one-point return. Even if there is no need right now to clean something up, someone might edit your function, allocate something and need to clean it up before return. If that happens it will be a nightmare job looking through all the return statements.

In C++ programming you have destructors and even now scope-exit guards. All these need to be here to ensure the code is exception-safe in the first place, so code is well guarded against early exit and therefore doing so has no logical downside and is purely a style issue.

I am not knowledgeable enough about Java, whether "finally" block code will get called and whether finalizers can handle the situation of needing to ensure something happens.

C# I certainly can't answer on.

D-language gives you proper built-in scope-exit guards and therefore is well-prepared for early exit and therefore should not present an issue other than style.

Functions should of course not be so long in the first place, and if you have a huge switch statement your code is probably also badly factored.

Early returns for-the-win. They can seem ugly, but much less ugly than big if wrappers, especially if there's multiple conditions to check.

I use both.

If DoSomething is 3-5 lines of code then the code just look beautiful using the first formatting method.

But if it has many more lines than that, then I prefer the second format. I don't like when the opening and closing brackets are not on the same screen.

A classic reason for single-entry-single-exit is that otherwise the formal semantics become unspeakably ugly otherwise (same reason GOTO was considered harmful).

Put another way, it's easier to reason about when your software will exit the routine if you have only 1 return. Which is also an argument against exceptions.

Typically I minimize the early-return approach.

Personally, I prefer to do pass/fail condition checks at the beginning. That allows me to group most of the most common failures at the top of the function with the rest of the logic to follow.

It depends.

Early return if there is some obvious dead end condition to check for right away that would make running the rest of the function pointless.*

Set Retval + single return if the function is more complex and could have multiple exit points otherwise (readability issue).

*This can often indicate a design problem. If you find that a lot of your methods need to check some external/paramater state or such before running the rest of the code, that's probably something that should be handled by the caller.

Use an If

In Don Knuth's book about GOTO's I read him give a reason for always having the most likely condition come first in an if statement. Under the assumption that this is still a reasonable idea (and not one out of pure consideration for the speed of the era). I'd say early returns aren't good programming practice, especially considering the fact that they're more often than not used for error handling, unless your code is more likely to fail than not fail :-)

If you follow the above advice, you'd need to put that return at the bottom of the function, and then you might as well not even call it a return there, just set the error code and return it two lines hence. Thereby achieving the 1 entry 1 exit ideal.

Delphi Specific...

I'm of the mind that this is a good programming practice for Delphi programmers, although I don't have any proof. Pre-D2009, we don't have an atomic way to return a value, we have exit; and result := foo; or we could just throw exceptions.

If you had to substitute

if (true) {
 return foo;
} 

for

if true then 
begin
  result := foo; 
  exit; 
end;

you might just get sick of seeing that at the top of every one of your functions and prefer

if false then 
begin
  result := bar;

   ... 
end
else
   result := foo;

and just avoid exit altogether.

I agree with the following statement:

I'm personally a fan of guard clauses (the second example) as it reduces the indenting of the function. Some people don't like them because it results in multiple return points from the function, but I think it's clearer with them.

Taken from this question in stackoverflow.

I use early returns almost exclusively these days, to an extreme. I write this

self = [super init];

if (self != nil)
{
    // your code here
}

return self;

as

self = [super init];
if (!self)
    return;

// your code here

return self;

but it really doesn't matter. If you have more than one or two levels of nesting in your functions, they need to be busted up.

I would prefer to write:

if(someCondition)
{
    SomeFunction();
}

Like you, I usually write the first one, but prefer the last one. If i have a lot of nested checks i usually refactor to the second method.

I don't like how the error handling is moved away from the check.

if not error A
  if not error B
    if not error C
      // do something
    else handle error C
  else handle error B
else handle error A

I prefer this:

if error A
  handle error A; return
if error B
  handle error B; return
if error C
  handle error C; return

// do something

The conditions on the top are called "preconditions". By putting if(!precond) return;, you are visually listing all preconditions.

Using the large "if-else" block may increase indent overhead (I forgot the quote about 3-level indents).

I prefer to keep if statements small.

So, choosing between:

if condition:
   line1
   line2
    ...
   line-n

and

if not condition: return

line1
line2
 ...
line-n

I'd choose what you described as "early return".

Mind you, I don't care about early returns or whatever, I just really like to simplify the code, shorten the bodies of if statements, etc.

Nested if's and for's and while's are horrible, avoid them at all costs.

As others say, it depends. For little functions that return values, I may code early returns. But for sizeable functions, I like to always have a place in the code where I know I can put something that will get executed before it returns.

I practice fail-fast at the function level. It keeps code consistent and clean (to me and those I've worked with). Because of that I always return early.

For some oft-checked conditions, you can implement aspects for those checks if you use AOP.

라이센스 : CC-BY-SA ~와 함께 속성
제휴하지 않습니다 softwareengineering.stackexchange
scroll top