Thursday, March 3, 2011

Only one return statement per method, even in this scenario?

Hi,

I like the idea of having only 1 return statement per method.

What do you do in this situation though?

public static string ChopText(string Text)
{
   if(String.IsNullOrEmpty(Text)
   {
      // return here ?????//
   }

}

The only alternative I can think of is setting a flag, and then checking for the flag.

Problem is, I don't like having huge if statements that span more than a page. I might ugly nested if statements also as a result of this.

From stackoverflow
  • Frankly, situations like this are why overly rigid rules are bad.

    The point of rules like this is to make code more readable and maintainable, so when they make the code harder to read they should be ignored.

    That doesn't mean that the rule should be discarded entirely, because most of the time it does make the code more readable.

  • Personally I think

    public static string ChopText(string Text))
    {
       if(String.IsNullOrEmpty(Text)
          return Text;
    
       ...
    }
    

    is totally fine, if you don't like those AND if it is getting big.

    James Brooks : I dunno, I'm not keen on mismatched parenthesis. (I kid)
  • It is OK to replace nested conditional with guard clauses.

    Blankman : nice link Marko!
    Will : I do this all the time. I think if your scope goes deeper than two levels you're doing something wrong...
    Marko Dumic : Yes, thanks. I encourage everyone to get themselves familiar with the practices presented on the site and to apply the ones they're comfortable with.
    danimajo : +1 Great! Thanks a lot
    PhiLho : Good! I should show that to colleagues swearing only by The Unique Return... :-)
  • You really have to weigh the costs and benefits of doing something like this. Will the benefits of having only one return statement outweigh the downsides of having to contort a method that should be fairly simple to write?

    I'd go with no for this case.

  • This is a case where the rule should be ignored. It is common to have several guard clauses at the entry point to the method that return or throw an ArgumentException if the arguments passed in are not in the correct format. Just keep these statements clustered at the beginning of your methods.

  • "I like the idea of having only 1 return statement per method." Explain?

    As soon as you know that the parameters passed to your method are invalid, you should return or throw an exception.

    Bad:

    if (Valid) { do lots of stuff}
    else {return};
    

    Clean:

    if (invalid) { return; }
    if (invalid2) {return; }
    

    Another approach:

    if (invalid) {
         throws IllegalArgumentException();
    
  • Code that tries to only have one return per function is far more complex. It's usually a rats nest of if-thens and assignments. I challenge you to look at that kind of code and know the correct value is always been returned from those different paths. No way.

    Having said that, large functions indicate you may need to refactor the code into smaller simpler functions anyway.

  • With exceptions in place there is no "single entry - single exit" rule any more anyway, so there is absolutely no need to follow a strict "one return statement" rule. Control flow can - theoretically - exit the function any time by throwing an exception and unwinding the stack, so even if you implement a strict "single entry - single exit" policy there is no guarantee that it will be followed at all.

    Feel free to exit a function at any time if it suits you!

  • The rule is antiquated. Plain and simple. It exists to help bad programmers write more readable code... and it backfired. Make your code small and readable, and get rid of that silly rule.

  • I strongly believe in "one entry / one exit", with the exception of validating inputs at the top of the function. That has to happen before the real work of the function begins though.

    I think this rule was intended to stop people from exiting in the middle of code that's doing real work simply because they think they're "done".

    The problem with multiple returns is that you can't be sure that required exit processing will be performed, such as closing a socket or releasing some other resource.

  • Your methods should in general terms not span more then "one screen". If they do you should (in general again) try to split them into several methods. This is probably far more important than to have "only one return statement"...

    After all, what we're looking for is readability.

  • I strongly disagree with the idea that "one entry one exit" results in complex code. I've just written an application for Blackberry and Android phones in Java that consists of roughly 100 files and there is not a single method that does not exit at the end. While it is a GUI app and multithreaded none of the code is complicated. Few if any routines are not visible in their entirety on a screen. I've been designing and writing software for 46 years in all but a handful of languages and operating systems, and for me "one entry one exit" makes for very simple, easy to read and maintain code. My $0.02 worth. Sorry if I've ruffled any feathers.

0 comments:

Post a Comment