0
Answered

Replace temp with query question?

Атанас Василев 6 years ago updated by Ankit Tanna 2 years ago 5

Considering https://refactoring.guru/home/slides/long-method/7/

Why is it better to use function on every call for the sum?

What's wrong with:

double calculateTotal() {
  double basePrice = basePrice();
  if (basePrice > 1000) {
    return basePrice * 0.95;
  }
  else {
    return basePrice * 0.98;
  }
}
double basePrice() {
  return quantity * itemPrice;
}


Why call the basePrice() multiple times, when its value can be cached? 

Answer

+4
Answer
Answered

Hi, Atanas.


Great question! In most cases, this refactoring is mandatory as a step towards the Extract Method which is extremely common and useful. Replace Temp with Query can eliminate a local variable that stands in the way of extracting code into separate method.

As with other performance issues, let it slide for the moment. Nine times out of ten, it won't matter. When it does matter, you will fix the problem during optimization. With your code better factored, you will often find more powerful optimizations, which you would have missed without refactoring. If worse comes to worse, it's very easy to put the temp back.

GOOD, I'M SATISFIED
Satisfaction mark by Атанас Василев 6 years ago
+4
Answer
Answered

Hi, Atanas.


Great question! In most cases, this refactoring is mandatory as a step towards the Extract Method which is extremely common and useful. Replace Temp with Query can eliminate a local variable that stands in the way of extracting code into separate method.

As with other performance issues, let it slide for the moment. Nine times out of ten, it won't matter. When it does matter, you will fix the problem during optimization. With your code better factored, you will often find more powerful optimizations, which you would have missed without refactoring. If worse comes to worse, it's very easy to put the temp back.

Hello, Alexander,
Thanks for the good explanation!

+4

I had the same query, and I understand now. 

I will further add to Alexander's well-explained answer. As he mentioned, 9/10 times we can use this refactoring strategy. I will add where not to apply this

1. If you have a database query operation involved 
2. if you are reading from a file
3. if anything involves with complexity greater than or equal to O(n^2)

Also, I think it is possible to calculate it once and store it as a class variable instead of a local variable. 

I agree with calculating only once using a method and storing the value because calling the same method 2 or 3 times is too much (especially for a calculation method that might make a call to the database calling a stored procedure or calculates based on complex business rules).

Extracting the method and calling 2 or 3 times made an optimized code less performant just for the sake of extracting a method without leaving the temp variable. Making queries all the time makes the code less performant every time, there's no doubt that this refactoring doesn't make the code cleaner or more optimized (the code is not cleaner, only that 1 line that optimized code was removed).

I also do not agree with the "optimize later" statement especially when you see the optimization problem beforehand because you can forget about the optimization by mistake and leave bad code.

So lets consider JavaScript language.

When I clarify my intent as basePrice being const, why should I be re-calcualting basePrice again and again unless the quantity is going to change(which I hope not).

 calculateTotal() {
  const basePrice = basePrice();
  if (basePrice > 1000) {
    return basePrice * 0.95;
  }
  else {
    return basePrice * 0.98;
  }
}

basePrice() {
  return quantity * itemPrice;
}