Replace Temp with Query

Question

The Replace Temp with Query refactoring method is recommended quite widely now but seems to be very inefficient for very little gain.

The method from the Martin Fowler's site gives the following example:

Extract the expression into a method. Replace all references to the temp with the expression. The new method can then be used in other methods.

 double basePrice = _quantity * _itemPrice;
 if (basePrice > 1000)
  return basePrice * 0.95;
 else
  return basePrice * 0.98;

becomes

 if (basePrice() > 1000)
  return basePrice() * 0.95;
 else
  return basePrice() * 0.98;


double basePrice() {
 return _quantity * _itemPrice;
}

Why is this a good idea? surely it means the calculation is needlessly repeated and you have the overhead of calling a function. I know CPU cycles are cheap but throwing them away like this seems careless?

Am I missing something?

Answer 1

It's intended to be more revealing of the intention of the code. In some cases it can be abused but not likely. for example you could update the queries for the 5% and 2% discounts with queries but in the name of the mehtods you could describe in the name reason for the discount. remember, it may be obvious today but in 6 months it might not be so - as I say - its not a matter if I forget, but when I forget.

  if (basePrice() > 1000)
     return bigTicketDiscount()
  else
     return regularDiscount()

double bigTicketDiscount(){
  return basePrice() * 0.95;
}

double regularDiscount(){
  return basePrice() * 0.98
}

Answer 2

It really seems to me that by doing this you are helping to make a single purpose to your original method; It no longer is responsible for calculating the basePrice and the discount price (or whatever it is that it is calculating). This is an example of sacrificing some CPU cycles to make the code more maintainable. The "wasted" cpu cycles are probably not something you'll notice for one and also it's possible the compiler could optimize it (maybe by noticing that the method is relatively simple and instead of calling the method it inserts the actual code (more like a macro). Anyway, these types of optimizations are best left to machines, leaving people with cleaner code to work on.

Answer 3

Having read Fowler's webpage on it, I can see no advantage to doing it that way. The only possible gain is by isolating an expression which may be used often into one spot, but that would be best handled by :

    double basePrice = basePrice();
    if (basePrice > 1000)
            return basePrice * 0.95;
    else
            return basePrice * 0.98;

Fowler offers no explanation why his revised code is better than the original, other than to read his book.

Answer 4

I think most of the utility of this refactoring does not come from applying it by itself but as a step towards an Extract Method which is extremely common and useful. On the cases where local variable stand in the way of Extract Method or complicate it by additional parameters, 'replace temp with query helps' to make Extract Method easier. Also in those cases, the query method that is generated gets reused by two different methods which are result of extract method.