Are there benefits to quick-exiting a method or constructor?

Question

I believe this to be related partially to short-circuiting logic, but I couldn't find any questions that directly answered my question. Possible related questions: Benefits of using short-circuit evaluation, Why use short-circuit code?

Consider the following two code blocks, both of which are possible constructors for a class

public MyClass(OtherClass other){
    if (other != null) {
       //do something with other, possibly default values in this object
    }
}

and this

public MyClass(OtherClass other){
    if (other == null)  return;

    //do something with other, possibly default values in this object
}

Is there any benefit to doing the latter over the former? There is no other code that follows in the constructor, just code that uses the other object to construct this one.

score 6 · Accepted Answer · answered Mar 28 '12 at 15:16

6

This a case where you should do what is the most readable to you and your coworkers. There is unlikely any perceivable speed difference between the two approaches.

answered Mar 28 '12 at 15:16

JaredPar

733,204
149
1,241
1,454

This was my thought too. Just wanted to see if there was a performance (or any other) benefit to doing it one way versus the other. – Thomas Jones Mar 28 '12 at 16:55

score 4 · Answer 2 · answered Mar 28 '12 at 15:15

4

The only difference is readability. With what you call "quick exit", you can stop thinking about what conditional you're inside of, because you're not inside of one anymore.

answered Mar 28 '12 at 15:15

John Saunders

160,644
26
247
397

score 3 · Answer 3 · answered Mar 28 '12 at 15:16

The latter is generally easier to read, and if you have multiple cases in which you would terminate then it gets even easier. There isn't a speed difference however.

Suppose you have a function that takes in Int32? and it exits if the value is either null, even, or greater than 100.

You could do

void fn( Int32? num ) {
    if ( num != null ) {
        if ( num < 100 ) {
            if ( num % 2 != 1 ) { 
                //method code

or something like

void fn( Int32? num ) {
    if ( num == null )
        return;

    if ( num > 100 )
        return;

    if (!(num % 2 != 1)) 
        return;

    //method code

Now the example is a bit silly, I can hear you now, Why not put them all on one line together with || or && and in this case yes. But imagine if the data validation were much more complicated than that? You'll end up with overly indented, much harder to read code.

score 1 · Answer 4 · answered Mar 28 '12 at 15:18

Specifically for this case, while you (assuming you are human) need to read through the rest of the source code to get to the end of the method, the processor does not. The block if statement in your first example evaluates the condition and if it evaluates to false, execution just jumps to the end of the method.

score 1 · Answer 5 · answered Mar 28 '12 at 15:21

I tried with this constructor

public AnotherExpense(string param)
    {
        if (param != null)
        {
            Console.WriteLine("test");
        }
    }

IL code

.method public hidebysig specialname rtspecialname 
    instance void  .ctor(string param) cil managed
{
  // Code size       20 (0x14)
  .maxstack  8
  IL_0000:  ldarg.0
  IL_0001:  call       instance void AccountParserCSV.Expense::.ctor()
  IL_0006:  ldarg.1
  IL_0007:  brfalse.s  IL_0013
  IL_0009:  ldstr      "test"
  IL_000e:  call       void [mscorlib]System.Console::WriteLine(string)
  IL_0013:  ret
} //

if you change it to

public AnotherExpense(string param)
    {
        if (param == null)
            return;

            Console.WriteLine("test");
    }

you get

.method public hidebysig specialname rtspecialname 
    instance void  .ctor(string param) cil managed
{
  // Code size       21 (0x15)
  .maxstack  8
  IL_0000:  ldarg.0
  IL_0001:  call       instance void AccountParserCSV.Expense::.ctor()
  IL_0006:  ldarg.1
  IL_0007:  brtrue.s   IL_000a
  IL_0009:  ret
  IL_000a:  ldstr      "test"
  IL_000f:  call       void [mscorlib]System.Console::WriteLine(string)
  IL_0014:  ret
}

see the 'difference' on line 7? ;-) edit - compiled in 'Release' with VS2010

I see the difference (one is evaluating against true, other false), but I dont exactly follow the flow of MSIL here (quite inexperienced at reading it). It looks like the latter evaluates to one more "line", in that theres two return statements, but the only difference is the GOTO on 007? — Thomas Jones, Mar 28 '12 at 17:25

score 0 · Answer 6 · answered Mar 28 '12 at 15:17

0

It's all about readability and consistency within your code base. A long method with returns sprinkled all over it would be hard to read (but then a long method is bad anyway). A couple of guard clauses at the start of the method can improve clarity and remove unnecessary nesting. However in the specific example you've given, you'd have to ask "why is null OK here?" Should you be throwing an exception, or passing in null objects instead?

answered Mar 28 '12 at 15:17

Ian Horwill

2,957
2
24
24

in this specific case, null is okay because they are convenience constructors for view models in asp.net mvc. If the database model from the service layer comes back as null (a good possibility for a create version of "Save"), the view model will be blank, and this helps to reduce Controller code by always instantiating a view model , regardless of if the database model is null. – Thomas Jones Mar 28 '12 at 17:23

score 0 · Answer 7 · answered Mar 28 '12 at 15:18

Readability is the biggest difference. In this case, the latter results in less indentation, which some might consider to be more readable. I also know a few developers who strongly believe that methods should have only a single return statement. Those developers would obviously prefer the former.

Are there benefits to quick-exiting a method or constructor?

7 Answers7