Adam S
Adam S

Reputation: 9235

Easy way to re-write multiple return statements

I have some code that's written like so:

private double function1()
{
    double result2 = function2();
    if (result2  < 0) { return result2; }

    double result3 = function3();
    if (result3  < 0) { return result3; }

    return 0;
}

I need to re-write it such that it has only one return statement. Is there an easy way to do this? It strikes me as inefficient to begin with that the same if construct is used twice. How can this inefficiency be cleaned up?

Upvotes: 3

Views: 1105

Answers (7)

Jeffrey L Whitledge
Jeffrey L Whitledge

Reputation: 59463

Here's the most readable version I can come up with within the constraints.

    private double Function1()
    {
        double result = 1;

        if (result >= 0)
            result = Function2();

        if (result >= 0)
            result = Function3();

        if (result >= 0)
            result = 0;

        return result;
    } 

This makes it blindingly obvious what is going on. And I'm pretty sure the optimizer will remove the first if expression anyway.

The advantage of this approach is that it is simple to add or re-prioritize the functions.

Upvotes: 1

Henk Holterman
Henk Holterman

Reputation: 273219

Keep it simple, keep it readable.

// direct conversion of your code
private double function1()
{
    double result = 0;

    double result2 = function2();
    if (result2  < 0) 
    {
      result =  result2; 
    }
    else
    {
       double result3 = function3();
       if (result3  < 0) 
       {
         result = result3; 
       }
    }

    return result;
}

A shorter version, probably easier on the eyes:

private double function1()
{
    double result = function2();

    if (result >= 0)   // if (!(result < 0))  to be safe for NaN
    {
       result = function3();
       if (result >= 0) 
       {
         result = 0; 
       }
    }    
    return result;
}

And

It strikes me as inefficient to begin with that the same if construct is used twice.

There is nothing inefficient about that. If the pattern was repeated (much) more you could start to think about an extra method or something to aid readability.

Upvotes: 7

Daniel Renshaw
Daniel Renshaw

Reputation: 34177

A single line solution that doesn't require altering function2/3. Not particularly readable but interesting:

private double function1()
{
    return new Func<double>[] { function2, function3 }
        .Select(c => c()).FirstOrDefault(c => c < 0);
}

Personally, I would opt for your original version.

Upvotes: 12

Romain Verdier
Romain Verdier

Reputation: 12971

I need to re-write it such that it has only one return statement

Are sure you really need that? The code shown in your question is perfectly valid and readable to me. Having multiple return statements is certainly not a problem here ; in fact it may be cleaner than all others alternatives (cf. answers).

Upvotes: 2

Scott Langham
Scott Langham

Reputation: 60381

how about:

delegate void double DoubleReturningFunction();

private double function1()
{
    DoubleReturningFunction[] functions = { function2, function3 };

    foreach( DoubleReturningFunction function in functions )
    {
       double result = function();
       if( result < 0 ) return result;
    }

    return 0;
}

Upvotes: 2

CaffGeek
CaffGeek

Reputation: 22054

You could just do this...

private double function1()
{
    var returnValue = 0;

    double result2 = function2();
    if (result2  < 0) { returnValue = result2; }

    double result3 = function3();
    if (result3  < 0) { returnValue = result3; }

    return returnValue;
}

But, that could have side effects, as now both function2 and function3 are always called. So, you'd have to have an else, and increase nesting like

private double function1()
{
    var returnValue = 0;

    double result2 = function2();
    if (result2  < 0) { 
        returnValue = result2; 
    } else {    
        double result3 = function3();
        if (result3  < 0) { returnValue = result3; }
    }

    return returnValue;
}

And then, it starts to smell more. If function 2 and 3 returned null instead of zero, you could just do this... some casting may be necessary to convert between decimal? and decimal

private double function1()
{
    return function2() ?? function3() ?? 0;
}

Upvotes: 1

D&#39;Arcy Rittich
D&#39;Arcy Rittich

Reputation: 171401

I actually prefer your original version with multiple returns. But, how about:

double ret = function2(); 
if (ret >= 0) 
    ret = function3();
if (ret > 0) 
    ret = 0;
return ret;   

Upvotes: 1

Related Questions