CODEWITHSUNDEEP
rubyrefactoring
user2914144
user2914144

Reputation: 137

How to refactor already simple ruby code

I want to refactor this ruby code

class UtilService


  def summer_start
    ...
  end

  def summer_end
    ...
  end 

  def calc(date, charge, quantity, winter_rate, winter_service_charge, summer_rate)
    if date < summer_start || date > summer_end
      chargeamt = quantity * winter_rate + winter_service_charge
    else
      chargeamt = quantity * summer_rate
    end
    chargeamt
  end
end

Since charge isn't being used I know I can get rid of that but other than getting rid of that one parameter I can't think of what else I can actually do.

I was also thinking of breaking up 'calc' into different methods

class UtilService
  def summer_start
  end

  def summer_end
  end

  def calc_summer_rate(date, quantity, summer_rate)
    if date > summer_start || date < summer_end
      chargeamt = quantity * summer_rate
    end
    chargeamt
  end

  def calc_winter_rate(date, quantity, winter_rate, winter_service_charge)
    if date < summer_start || date > summer_end
      chargeamt = quantity * winter_rate + winter_service_charge
    end
    chargeamt
  end

end

But it doesn't seem like my refactoring makes much sense.

Upvotes: 0

Views: 53

Answers (2)

Cary Swoveland
Cary Swoveland

Reputation: 110675

I would be inclined to write something like the following.

rates = { winter: { per_unit: 123, service_charge: 456 },
          summer: { per_unit: 135, service_charge: 0   } }

def calc(date, quantity, rates)
  rate = summer?(date) ? rates[:summer] : rates[:winter]
  quantity * rate[:per_unit] + rate[:service_charge]
end

def summer?(date)
  date.between?(summer_start, summer_end)
end

Upvotes: 3

Gerry
Gerry

Reputation: 10497

I don't see big refactoring needed, maybe just moving the is summer? logic to another method and removing unnecesary chargeamt variable; something like this:

class UtilService
  def summer_start
  end

  def summer_end
  end

  def calc(date, quantity, winter_rate, winter_service_charge, summer_rate)
    if summer?(date)
      quantity * winter_rate + winter_service_charge
    else
      quantity * summer_rate
    end
  end

  private
  def summer?(date)
    date > summer_start || date < summer_end
  end
end

Upvotes: 2

Related Questions