How to deal with variables when extracting methods in to smaller methods?

Question

This is an abstract question to clarify a refactoring concept in the ruby language. Assume in the real world that there would be many more variables and method in the Furniture Class and Refinish method.

I have a Class called Furniture.

It has a method called 'refurnish(VarnishToUse, TimeToTake)'. This method has

varnish_cost = Varnish(VarnishToUse).price * TimeToTake

I want to extract out the cost calculation into

def VarnishCost
 price * TimeToTake
end

then the Furniture class can just have:

def Refurnish(VarnishToUse)
 varnish_cost = VarnishCost(VarnishToUse)
end

How do I pass in the VarnishToUse into VarnishCost? Should the method be

def VarnishCost(Varnish)
...
end

or should I be setting an instance variable @varnish = VarnishToUse in the Refinish class and then just expect that instance variable to be available in the VarnishCost method as I showed above? Does it make a difference if the other method is private? protected?

I am looking to minimize the use of variables and multiple responsibilities for a class (SRP).

• Manfred: I'd just pass 'Varnish' as the parameter into the VarnishCost() method. However, there can be a gazillion other aspects that may play a role and influence your decision. (Hence this as a comment and not as an answer.)
• Michael Durrant: What if I have 4 other parameters? I know that more than 2 or three parameters is bad practice (number of combos to test). So what would be good practice?
• blueberryfields: In these kinds of situations, I tend to optimize for the least amount of written code as a rough approximation for the desired result.
• Manfred: If the number of parameters becomes larger and you find similar combinations of parameters passed around that you might have identified a candidate for a class. Once turned into a class you can reduce the number of parameters again because you have found a higher level of abstraction. This doesn't work always but can be an indication for improving your domain model.

Answer 1

If these are all methods in the Furniture class, you don't need to "pass" anything, just access the methods:

class Furniture
 attr_accessor :time_to_take
 attr_accessor :varnish_to_use
 def varnish_cost
 varnish_to_use.price * time_to_take
 end
 def refurnish
 # varnish_cost * markup_percent + premium # if you want to affect the price more...
 varnish_cost
 end
end
class Varnish
 def self.price
 # default price
 10
 end
end
class BasicVarnish < Varnish
 def self.price
 5
 end
end
class PremiumVarnish < Varnish
 def self.price
 20
 end
end
so now:
f1 = Furniture.new(:time_to_take => 2, :varnish_to_use => BasicVarnish)
f1.refurnish => 10
f2 = Furniture.new(:time_to_take => 2, :varnish_to_use => PremiumVarnish)
f1.refurnish => 40

(PS I changed the names of the methods as it's a bit confusing having them as Constants)

• Michael Durrant: could you expand a little more on the use of attr_accessor :varnish_to_use and how that will relate to getting price with varnish_to_use.price
• Pavling: attr_accessor is a Ruby method of short-cutting getter and setter methods. In the object I outlined, it's just part of the interface that relates a Varnish instance to a Furniture instance. Then you can access the Varnish's methods, such as ".price".

Answer 2

should I be setting an instance variable @varnish = VarnishToUse in the Refinish class

You should do this when there are some other functions in your Refinish class which can use varnish, too, and when having a somewhat "global" (in the context of your class) variable in your code does not give you a higher risk of malfunction (for example, when varnish does not change any more after first initialization). Especially the second condition should be fulfilled, otherwise pass VarnishToUse as a parameter.