ChatGPT解决这个技术问题 Extra ChatGPT

Is it bad practice to use return inside a void method?

Imagine the following code:

void DoThis()
{
    if (!isValid) return;

    DoThat();
}

void DoThat() {
    Console.WriteLine("DoThat()");
}

Is it OK to use a return inside a void method? Does it have any performance penalty? Or it would be better to write a code like this:

void DoThis()
{
    if (isValid)
    {
        DoThat();
    }
}
What about: void DoThis() { if (isValid) DoThat(); }
imagine the code? Why? It's right there! :-D
This is good question, i always think is it good practice to use return; to exit the method or function. Especially in a LINQ data mining method having multiple IQueryable result and all of them depend on each other. If one of them have no result, alert and exit.

C
Community

A return in a void method is not bad, is a common practice to invert if statements to reduce nesting.

And having less nesting on your methods improves code readability and maintainability.

Actually if you have a void method without any return statement, the compiler will always generate a ret instruction at the end of it.


J
Jason Williams

There is another great reason for using guards (as opposed to nested code): If another programmer adds code to your function, they are working in a safer environment.

Consider:

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }
}

versus:

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
}

Now, imagine another programmer adds the line: obj.DoSomethingElse();

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }

    obj.DoSomethingElse();
}

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
    obj.DoSomethingElse();
}

Obviously this is a simplistic case, but the programmer has added a crash to the program in the first (nested code) instance. In the second example (early-exit with guards), once you get past the guard, your code is safe from unintentional use of a null reference.

Sure, a great programmer doesn't make mistakes like this (often). But prevention is better than cure - we can write the code in a way that eliminates this potential source of errors entirely. Nesting adds complexity, so best practices recommend refactoring code to reduce nesting.


Yes, but on the other hand, several layers of nesting, with their conditions, make the code even more prone to bugs, the logic more difficult to track and - more importantly - harder to debug. Flat functions are lesser evil, IMO.
I'm arguing in favour of reduced nesting! :-)
I agree with this. Also, from a refactor stand point, it's easier and safer to refactor the method if obj becomes a struct or something you can guarantee isn't going to be null.
C
Community

Bad practice??? No way. In fact, it is always better to handle validations by returning from the method at the earliest if validations fail. Else it would result in huge amount of nested ifs & elses. Terminating early improves code readability.

Also check the responses on a similar question: Should I use return/continue statement instead of if-else?


M
Mike Hall

It's not bad practice (for all reasons already stated). However, the more returns you have in a method, the more likely it should be split into smaller logical methods.


Sometimes the returns ARE based on the results of other smaller logical methods. eg. isSomeCondition = IsSomeCondition(someCriteria) if(!isSomeCondition) return; isSomeOtherCondition = IsSomeOtherCondition(someOtherCriteria) if(!isSomeOtherCondition) return;
c
cdmckay

The first example is using a guard statement. From Wikipedia:

In computer programming, a guard is a boolean expression that must evaluate to true if the program execution is to continue in the branch in question.

I think having a bunch of guards at the top of a method is a perfectly understandable way to program. It is basically saying "do not execute this method if any of these are true".

So in general it would like this:

void DoThis()
{
  if (guard1) return;
  if (guard2) return;
  ...
  if (guardN) return;

  DoThat();
}

I think that's a lot more readable then:

void DoThis()
{
  if (guard1 && guard2 && guard3)
  {
    DoThat();
  }
}

R
Russell

There is no performance penalty, however the second piece of code is more readable and hence easier to maintain.


Russell i disagree with your opinion but you shouldn't have been down-voted for it. +1 to even it out. Btw, i believe that a boolean test and return on a single line followed by a blank line is a clear indication of what's going on. e.g. Rodrigo's first example.
I disagree with this. Increasing nesting does not improve readability. The first piece of code is using a "guard" statement, which is a perfectly understandable pattern.
I disagree as well. Guard clauses which bail out of a function early are generally considered a Good Thing nowadays in helping a reader to understand the implementation.
I
Imagist

In this case, your second example is better code, but that has nothing to do with returning from a void function, it's simply because the second code is more direct. But returning from a void function is entirely fine.


N
Noon Silk

It's perfectly okay and no 'performance penalty', but never ever write an 'if' statement without brackets.

Always

if( foo ){
    return;
}

It's way more readable; and you'll never accidentally assume that some parts of the code are within that statement when they're not.


readable is subjective. imho, anything added to code that is unnecessary makes it less readable... (I have to read more, and then I wonder why it's there and waste time trying to make sure I'm not missing something) ...but that's my subjective opinion
The better reason to always include the braces is less about readability and more about safety. Without the braces it is all to easy for some one later to fix a bug that requires additional statements as part of the if, not pay close enough attention and add them without also adding the braces. By always including the braces, this risk is eliminated.
Silky, please press enter before your {. This lines up your { with your } in the same column, which greatly aids readability (much easier to find corresponding open/close braces).
@Imagist I'll leave that to personal preference; and it's done the way I prefer :)
If every close-brace is matched with an open-brace which is placed at the same level of indent, then visually distinguishing which if statements require close braces will be easy, and thus having if statements control a single statement will be safe. Pushing the open-brace back to the line with the if saves a line of vertical space on each multi-statement if, but will require using an otherwise-unnecessary close-brace line.
J
John R. Strohm

I'm going to disagree with all you young whippersnappers on this one.

Using return in the middle of a method, void or otherwise, is very bad practice, for reasons that were articulated quite clearly, nearly forty years ago, by the late Edsger W. Dijkstra, starting in the well-known "GOTO Statement Considered Harmful", and continuing in "Structured Programming", by Dahl, Dijkstra, and Hoare.

The basic rule is that every control structure, and every module, should have exactly one entry and one exit. An explicit return in the middle of the module breaks that rule, and makes it much harder to reason about the state of the program, which in turn makes it much harder to say whether the program is correct or not (which is a much stronger property than "whether it appears to work or not").

"GOTO Statement Considered Harmful" and "Structured Programming" kicked off the "Structured Programming" revolution of the 1970s. Those two pieces are the reasons we have if-then-else, while-do, and other explicit control constructs today, and why GOTO statements in high-level languages are on the Endangered Species list. (My personal opinion is that they need to be on the Extinct Species list.)

It is worth noting that the Message Flow Modulator, the first piece of military software that EVER passed acceptance testing on the first try, with no deviations, waivers, or "yeah, but" verbiage, was written in a language that did not even have a GOTO statement.

It is also worth mentioning that Nicklaus Wirth changed the semantics of the RETURN statement in Oberon-07, the latest version of the Oberon programming language, making it a trailing piece of the declaration of a typed procedure (i.e., function), rather than an executable statement in the body of the function. His explication of the change said that he did it precisely because the previous form WAS a violation of the one-exit principle of Structured Programming.


@John: we got over the Dykstra injunction about multiple returns just about the time we got over Pascal (most of us, anyway).
Cases where multiple returns are required are frequently signs that a method is trying to do too much and should be pared down. I'm not going to go as far as John with this, and a return statement as part of parameter validation may be a reasonable exception, but I get where the idea is coming from.
@nairdaen: There is still controversy over exceptions in that quarter. My guideline is this: If the system under development MUST fix the problem that caused the original exceptional condition, and I don't mind pissing off the guy who will have to write that code, I'll throw an exception. Then I get yelled at in a meeting, because the guy didn't bother to catch the exception, and the app crashed in testing, and I explain WHY he has to fix the problem, and things settle down again.
There is a big difference between guard statements and gotos. The evil of gotos is that they can jump anywhere, so can be very confusing to unravel and remember. Guard statements are the exact opposite - they provide a gated entry to a method, after which you know that you are working in a "safe" environment, reducing the number of things you have to consider as you write the rest of the code (e.g. "I know this pointer will never be null, so I don't need to handle that case throughout the code").
@Jason: The original question was not specifically about guard statements, but about random return statements in the middle of a method. The example given example appeard to be a guard. The key issue is that, at the return site, you want to be able to reason about what the method did or didn't do, and random returns make that harder, for EXACTLY the same reasons that random GOTOs make it harder. See: Dijkstra, "GOTO Statement Considered Harmful". On the syntax side, cdmckay gave, in another answer, his preferred syntax for guards; I disagree with his opinion of which form is more readable.
l
lue

While using guards, make sure you follow certain guidelines to not confuse readers.

the function does one thing

guards are only introduced as the first logic in the function

the unnested part contains the function's core intent

Example

// guards point you to the core intent
void Remove(RayCastResult rayHit){

  if(rayHit== RayCastResult.Empty)
    return
    ;
  rayHit.Collider.Parent.Remove();
}

// no guards needed: function split into multiple cases
int WonOrLostMoney(int flaw)=>
  flaw==0 ? 100 :
  flaw<10 ? 30 :
  flaw<20 ? 0 :
  -20
;

D
Dhananjay

Throw exception instead of returning nothing when object is null etc.

Your method expects object to be not null and is not the case so you should throw exception and let caller handle that.

But early return is not bad practice otherwise.


The answer does not answer the question. The question is a void method so there is nothing being returned. Also, the methods don't have parameters. I get the point of not returning null if the return type is an object but that does not apply to this question.