4

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

|improve this question
  • 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.) – Manfred Feb 24 '12 at 6:03
  • 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? – Michael Durrant Feb 24 '12 at 6:36
  • In these kinds of situations, I tend to optimize for the least amount of written code as a rough approximation for the desired result. – blueberryfields Feb 24 '12 at 6:50
  • 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. – Manfred Feb 24 '12 at 8:58
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)

|improve this answer
  • 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 – Michael Durrant Feb 25 '12 at 6:33
  • 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". – Pavling Feb 27 '12 at 8:43
1

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.

|improve this answer

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.