Imagine a concrete example of a code which, given a price of an item, should call a method while specifying if the item is a rebate (price inferior to zero), a paid product (price superior to zero) or a free product (price equal to zero).
Your two pieces of code become:
Solution 1:
if (price < 0)
{
this.DoSomething(PriceType.Rebate);
}
else if (price > 0)
{
this.DoSomething(PriceType.Product);
}
else // price == 0
{
this.DoSomething(PriceType.Free);
}
Solution 2:
priceType = PriceType.Free;
if (price < 0)
{
priceType = PriceType.Rebate;
}
else if (price > 0)
{
priceType = PriceType.Product;
}
this.DoSomething(priceType);
The second solution, as is, has less lines (10 versus 12), despite your assertion that "it takes up more lines of code", but the first one looks more readable. On the other hand, the second solution can be refactored to become much more readable:
Refactoring of solution 2:
private PriceType FindPriceType(price)
{
if (price < 0)
{
return PriceType.Rebate;
}
else if (price > 0)
{
return PriceType.Product;
}
return PriceType.Free;
}
...
this.DoSomething(this.FindPriceType(price));
By separating the original method into two methods, the code becomes more readable through the introduction of an explicit name of the method.
There are also two specific cases where the second solution allows even more benefits:
Specific cases
The first case is the one where all conditions have the form: if (something == value)
. For instance:
if (priceType == 1)
{
return PriceType.Rebate;
}
if (priceType == 2)
{
return PriceType.Product;
}
if (priceType == 3)
{
return PriceType.Free;
}
throw new NotImplementedException();
can be transformed into a much shorter:
var map = {
1: PriceType.Rebate,
2: PriceType.Product,
3: PriceType.Free
}[priceType];
A second specific case is where you have not three branches, but only one if/else
. Example:
Solution 1:
if (price < 0)
{
this.DoSomething(PriceType.Rebate);
}
else
{
this.DoSomething(PriceType.Product);
}
Solution 2:
priceType = PriceType.Product;
if (price < 0)
{
return PriceType.Rebate;
}
this.DoSomething(priceType);
Here, the second solution can (in many languages) be transformed into a one-liner:
this.DoSomething(price < 0 ? PriceType.Rebate : PriceType.Product);