(Offline) Smell Exercise

What are smells?

• Summary of Code Smells
• Feel free to google for code smells, you will find lots of information.

Preparation

1. On your own, take a look at the code below. You can copy it into your own text editor
2. Get it to run, and understand what it does. You might have to experiment a little and learn some new Ruby.
3. Study the code. Concentrate on two code smells:
4. The unusual way that params.each works (line 3). Try to figure out what exactly that line is doing. Yes it’s pretty clever but why did I write it that way, and what are some problems with that?
5. The convoluted way that price is computed (line 27+). Generally when you have a nested calculation like that, and secondly when a class has some kind of “code” to distinguish types, this is a code smell and tells us that there’s room for improvement.
6. And of course, this method is much longer than it should be.

Instructions

1. On your own, try to come up with how to improve the code. You will be doing some “refactoring” which is a fancy way to say changing the code that is written without changing it’s functionality in any way. Take 30 minutes or less doing this.
2. Find 1 or 2 other students in our class to work with. This can be in person or over email. Discuss each person’s views on the smells. Why are they a problem? How could they be improved? Together write the best improved version of this program using your best ideas.
3. Write up a joint summary (PDF) of your work (remember to include your names!). Discuss: * The problems you all saw with the original code * What new Ruby you had to learn to accomplish this * What does your improved code looks like? * In what ways do you feel it’s a solution?

Here’s the code:

 class MountainBike
 
   TIRE_WIDTH_FACTOR = 250
   FRONT_SUSPENSION_FACTOR = 100
   REAR_SUSPENSION_FACTOR = 150
 
   def initialize(params)
     params.each { |key, value| instance_variable_set "@#{key}", value }
 
    @commission = 0.25
    @front_suspension_price = 95.0
    @rear_suspension_price = 67.0
    @base_price = 490.00
  end

  def off_road_ability
    result = @tire_width * TIRE_WIDTH_FACTOR
    if @type_code == :front_suspension || @type_code == :full_suspension
      result += @front_fork_travel * FRONT_SUSPENSION_FACTOR
    end
    if @type_code == :full_suspension
      result += @rear_fork_travel * REAR_SUSPENSION_FACTOR
    end
    result
  end
  
  def price
    case @type_code
    when :rigid
      (1 + @commission) * @base_price
    when :front_suspension
      (1 + @commission) * @base_price + @front_suspension_price
    when :full_suspension
      (1 + @commission) * @base_price + @front_suspension_price +
        @rear_suspension_price
    end
  end

  def owner
    @owner
  end

  def to_s
    "Mountain bike - owner: #{owner}, off road ability: #{off_road_ability()}, price: #{price}"
  end

end

pitos_bike = MountainBike.new(:owner => "Pito", :type_code => :rigid, :tire_width => 2.5)
puts pitos_bike

ricks_bike = MountainBike.new(:owner => "Rick", :type_code => :front_suspension, :tire_width => 2, :front_fork_travel => 3)
puts ricks_bike