2

I am reading Martin Fowlers Refactoring book. On page 110 he talks about 'Replace Method with Method Object' where he turns a method with local variables into a class with instance variables.

The outcome is a class called: PriceCalculator with three instance variables i.e. primaryBasePrice, secondaryBasePrice and teritary base price.

I recently asked a question about local variables and instance variables: When should local variables be used over instance variables?. I agree with Samuels answer and Timothy Truckles' answer - I have always used the principle of least visibility in my work.

However, Fowlers refactoring seems to contradict this and is confusing me. Also all the class diagrams in my UML book seem to contradict the principle of least visibility i.e. they use a class with instance variables regardless of the scenario like this one: http://www.newthinktank.com/wp-content/uploads/2012/12/Object-Oriented-Design.png. I understand that you have to compromise in this game when considering performance etc. Is it "better" design to have instance variables rather than local variables?

  • 2
    Take Martin Fowler's advice with a grain of salt. He is overall a great author, but sometimes he spills out some stuff that makes little sense (like his position on anemic domain models). – T. Sar Jun 22 '17 at 10:49
  • @TSar, but a lot of books appear to be the same. For example this one: newthinktank.com/wp-content/uploads/2012/12/…. I wander if they are written by Business Analysts who perhaps don't understand the practicalities like a developer. – w0051977 Jun 22 '17 at 10:52
  • 3
    Thanks for the "newthinktank" link. It made my eyes bleed looking at the way it takes a trivial requirement and turns it into a monstrosity of over-engineering. I'll use that as evidence for why a very deep hole should be dug, UML thrown into it, followed by lots of super-hard reinforced concrete, so it never gets used again. – David Arno Jun 22 '17 at 11:23
  • 1
    @w0051977 Those examples make one thing that they are the "right way to do everything", when in fact they should be used for a specific set of problems - like every other way of making code. They have their pros and their cons, and sometimes they are the blatant wrong choice for something - like in the examples they are trying to illustrate. – T. Sar Jun 22 '17 at 11:46
  • 1
    That's why me, @DavidArno, and several other uses are becoming very vocal around the site regarding anemic models, design patterns, and other stuff. There is a lot of misinformation and cargo cult programming being tossed around without people really stopping to thing if what a given book's author or blog post is useful or even correct. – T. Sar Jun 22 '17 at 11:49
4

A refactoring is not necessarily an improvement, it is just a structural change. Turning a method into a method object is a refactoring, but so is the opposite, turning a method object into a plain method.

Refactorings are often steps performed in order to make it possible to introduce new features. For example, if you need to introduce the ability to undo or replay select methods, then a first step might be to turn the methods into method-objects, since this will allow you to implement the command pattern as the next step.

But in isolation, turning a method into a method object will not improve your code.

  • Thanks. Is there a term for turning a method object into a plain method? – w0051977 Jun 22 '17 at 20:11
  • @w0051977: sure, Inline class – Doc Brown Jun 22 '17 at 20:13
2

To cite Timothy Truckels' correctly:

Turning local variables into instance variables (for no particular reason) violates the least visibility scope principle

But what is a "particular reason"? I think you will agree a sensible reason is when a method exceeds a certain complexity. And that is what Fowlers "Replace Method by Method Object" refactoring is for, no less, no more. Opposed to @TSar, I don't find Fowler's example in "Replace Method with Method Object" particulary bad. He clearly states it is for "a long method that uses local variables in such a way that you cannot apply Extract Method" - which is a clear difference to the Calculator example in your former question. Note also Fowler's catalogue tries to give a comprehensive description for each of the refactorings, he surely did not want to fill five pages in his book with the code from a real program to make sure anyone believes the method he turns into a class is really a long, complex one.

If you take the "least visibility scope principle" to the extreme just because you want to follow it in a cargo cult manner, you get either very long methods with hundreds of LOC and dozens of local variables, just for not introducing instance variables. Or you get clusters of methods with dozens of parameters, passing the local variables from one method to another, for the same reason. That is surely not what you want to achieve.

  • 2
    This is all very good, however the other extreme is also true - replacing small and simple method by a class is also really bad. This is, like almost everything on software development, part of a spectrum of viable solutions which should be weighted for the problem at hand before being picked up, instead of because someone said X or Y. Fowler's example is bad because it gives the impression you should use this for trivial things. – T. Sar Jun 22 '17 at 11:51
  • @TSar: well, I don't know which of Fowler's example you are referring to, but in my copy of the "Refactoring" book he speaks clearly about a long method with a long computation. – Doc Brown Jun 22 '17 at 14:00
  • For me the "particular reason" is because you want that local variable to become a part of the state of your object even if it is currently used only by one method. – Walfrat Jun 22 '17 at 14:17
  • 1
    @DocBrown This example is bad because it shows a very simplified class and UML diagram to exemplify his point. "Long computation" (as in, takes a while to finish) isn't a good reason to do this type of thing - lots of parameters, need to check the state while the thing runs, shared memory space, etc - those things are good reasons. By reducing the problem to a example that looks trivial, it pushes the idea that this technique should be used on trivial stuff. He is 100% correct that this is a good way to refactor stuff... – T. Sar Jun 22 '17 at 14:26
  • ... and it should be done for the correct reasons. His didactic is misleading because he doesn't help a novice dev to find the correct reasons to do so. – T. Sar Jun 22 '17 at 14:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.