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).

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)

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.