A Refactoring Nit
Here’s a ‘before’ and ‘after’ example from the book Refactoring: Improving the Design of Existing Code (Safari link).
double basePrice = _quantity * _itemPrice;
if (basePrice > 1000)
return basePrice * 0.95;
else
return basePrice * 0.98;
if (basePrice() > 1000)
return basePrice() * 0.95;
else
return basePrice() * 0.98;
…
double basePrice() {
return _quantity * _itemPrice;
}
The author is suggesting that by making basePrice() a method that you call instead of the temporary variable, it’s easier to move the code around, since the scope of the temporary isn’t an issue anymore.
This is fine if your only goal is to make refactoring your code easier, but there are two points worth mentioning:
- Method calls are more expensive than referencing temporary variables. A temporary variable is a cache that saves you from having to call the function over and over. Your optimizer may help you here, or it may not – and even if it does today, someone might come in tomorrow and make the basePrice() function do a database lookup or some other longer operation, which would result in the program being much slower than the non-refactored version.
- Imagine the basePrice() function does get replaced with one that looks the price up in a database. It’s quite possible now that basePrice will return different values in the two separate calls, and mess up the program’s logic.
These are the sorts of subtle problems you can introduce while refactoring if you’re not careful. I’m a big fan of refactoring, but I’ve seen it go wrong as well as right, so some caution is definitely warranted, especially with the new ease of refactoring that new tools like Whidbey is bringing us.