Reputation: 9235
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
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
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
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
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
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
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
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