← Back to Upcase

Refactoring - Extract Method


(Upcase ) #1

This topic is for the [Extract Method][] exercise in the [Refactoring][] trail. Post any questions, corrections, or pointers you have to share with other Upcase subscribers.
[Extract Method]: https://exercises.upcase.com/exercises/extract-method
[Refactoring]: https://thoughtbot.com/upcase/refactoring


(Tom Freudenheim) #2

I looked at several solutions to this, and they all were different.

I get that refactoring is an art, not a science, and that there may be better and worse solutions but no correct solution. However, I would like some input on what is a good solution, beyond the comments I may get on my proposal.

For instance, one piece that seemed to be challenging was the reduce statement that does two things – output the items and get the subtotal. Many solutions tried to separate these, but ran into challenges when they wanted to avoid iterating through the items twice. My take on this is that it seems like pre-mature optimization, though I don’t know what kind of items will be passed into the class.

Another thing I saw some people do was making a subtotal method which cached the result of the loop in a property. Is that a pattern or anti-pattern? Is there a need to initialize that cache on every call to print, in case the subtotal may change at some point? Is it maybe just a clear to use a temporary variable in a case like this?

I would love to see a “correct” solution and be able to discuss it.