3

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?

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
w0051977
  • 7,031
  • 6
  • 59
  • 87
  • 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: http://www.newthinktank.com/wp-content/uploads/2012/12/Object-Oriented-Design.png. 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
  • Possible duplicate of [Should I create a class if my function is complex and has a lot of variables?](https://softwareengineering.stackexchange.com/questions/297090/should-i-create-a-class-if-my-function-is-complex-and-has-a-lot-of-variables) – gnat Jun 22 '17 at 10:54
  • 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
  • @David Arno, are you saying that you dislike the code in the link? It looks highly cohesive and loosely coupled to me. A little over engineered, but gets the point across. – w0051977 Jun 22 '17 at 11:27
  • @w0051977 The problem with with several theoretical ways of making code is that most often than note those "suggestions" are overkills. Using the examples provided on Martin Fowler's book or on the New Think Thank, one would think that you should use that level of engineering for things like a price calculator or a coin game. You shouldn't. Those are bad examples. This type of example is what making a bunch of people nowadays use a DI Framework or a set of microservices to write a "Hello World" program. – T. Sar Jun 22 '17 at 11:44
  • 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
  • @DocBrown [He is refering to this refactoring](https://refactoring.com/catalog/replaceMethodWithMethodObject.html). – T. Sar Jun 22 '17 at 14:56
  • @DocBrown The OP is even citing the exact names used in the examples and I'm referring too. I think it's pretty clear that he is referring to this one. I'm in favor in waiting a bit for the OP to clarify, however. – T. Sar Jun 22 '17 at 17:03

2 Answers2

5

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.

JacquesB
  • 57,310
  • 21
  • 127
  • 176
3

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.

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • 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](https://refactoring.com/catalog/replaceMethodWithMethodObject.html) 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
  • @TSar: well, I read "long computation" as "complex computation". And Fowler's example surely does not become better if one adds half a dozen methods to this class just to demonstrate it is complex enough to justify the usage of a class instead of a method. And I agree to you, replacing small and simple method by a class is also really bad, this is the kind of example the OP gave in the first [link](https://softwareengineering.stackexchange.com/questions/350942/when-should-local-variables-be-used-over-instance-variables/350944). – Doc Brown Jun 22 '17 at 14:33