Sunday 18 October 2009 @ 8:33 am
Well, now I know. Here is an example of some code I just wrote:
if (doDeficitCalc)
{
throw
new ApplicationException("Need to implement deficit weight rating");
}
else
{
// Linear interpolation from top of range to bottom for speed
double x1 = rateBreakList[0];
double x2 = rateBreakList[rateBreakList.Count - 1];
double y1 = rateList[0];
double y2 = rateList[rateList.Count - 1];
double frac = weight/(x2 - x1);
double yResult = y1 + (frac*(y2 - y1));
retWeight = yResult;
}
ReSharper correctly warns me that the else statement is redundant as shown below:
So, if I hover over the “if” statement, I get the chance to invert the if clause
When I accept the invert if, I now get:
if (!doDeficitCalc)
{
// Linear interpolation from top of range to bottom for speed
double x1 = rateBreakList[0];
double x2 = rateBreakList[rateBreakList.Count - 1];
double y1 = rateList[0];
double y2 = rateList[rateList.Count - 1];
double frac = weight/(x2 - x1);
double yResult = y1 + (frac*(y2 - y1));
retWeight = yResult;
}
else
{
throw
new ApplicationException("Need to implement deficit weight rating");
}
Which has no warning!!! Just what I wanted with less key strokes.
Thank you again ReSharper!
October 18th, 2009 at 1:49 pm
I would follow the first warning (redundant if) because it would reduce nesting… when execution enters the “if” the routine will return on the throw anyway.
if(!doDeficitCalc)
throw new…
double x1…
October 19th, 2009 at 7:25 am
I’m with Frederico. Redundant Else for the win! The invert refactoring is useful in a number of cases, one being to allow you to apply redundant else when what was the else block always returns, or when the else block is much shorter than the if block (which, when swapped to be first, reads more easily), or when the if condition is more easily read when inverted (due to negations, ands versus ors, etc.), and so on. Lotsa great uses out there.
October 19th, 2009 at 7:44 am
I totally get Frederico and Jeremy’s points. My feeling in general is, for maintainability, I like to be as explicit as possible. By having the else at the bottom of an if makes it 100% clear. Otherwise, someone might add something to the bottom of the method and not realize it was only meant to be executed when the if failed (which might not be obvious from the logic like it is in my example). Kind of feels like the whether to use {} debate which maybe I will post next.
October 19th, 2009 at 8:25 am
[...] my last post, I found a use for inverting if statements. That is, I postulated that it is better to have [...]
October 21st, 2009 at 1:29 am
I go with Frederico.
The if(doDeficitCalc) throw new … is a guard statement, and really shouldn’t have an else.
Also, that second bit should probably be refactored to another function
November 9th, 2009 at 7:17 pm
@DavidKemp Agreed!
The total give away is the comment:
// Linear interpolation from top of range to bottom for speed
Why write that comment when you can just make a
DoTopToBottomLinearInterpolation() method?
November 10th, 2009 at 10:16 pm
[...] my last post, I found a use for inverting if statements. That is, I postulated that it is better to have [...]
March 5th, 2010 at 5:12 pm
Redundant else surely means you should remove the else, rather than you should negate your condition and put the statement in an ‘if’.
Use the if as a guard statement, throw an exception if met, and then continue. There is no need for an else here except confusion (‘when is this code *not* executed?)