← Back to Upcase

Where to start refactoring a large class

(Jarrett Green) #1

I’ve been reading Ruby Science and it’s been eye opening, and well, depressing, lol.

We have a large application that builds budgets for our clients. You can create a number of scenarios, throw employees in there, and mess with numbers. Our main model is called ScenarioEntry.

class ScenarioEntry < ActiveRecord::Base
  belongs_to :scenario
  belongs_to :employee
end

We have a TON of private methods to do a lot of math. There are a ton of conditionals in each method based on the state of the scenario. for instance:

  def non_prorated_lump_sum

  #### Salaried - Already Over Max
  if employee.annual? and maxed_out?
    return (pristine_new_wages - current_wages)*effective_date_to_end_of_year_ratio
  end  

  #### Salaried - Taken Over Max
  if employee.annual? and taken_to_max?
    return (pristine_new_wages - new_wages)*effective_date_to_end_of_year_ratio
  end

  #### Hourly - Already Over Max
  if !employee.annual? and maxed_out?
    return ((pristine_new_wages - current_wages)*hours_worked_after_adjusted_date)
  end

  #### Hourly - Taken to Max
  if !employee.annual? and taken_to_max?
    return ((pristine_new_wages - new_wages)*hours_worked_after_adjusted_date)
  end
end

I want to take these situations, and moved them out somehow - as a separate class for each, or module for each or something. So the methods and math for an hourly employee who has a lump sum payment due and is over their maximum wage to be in hourly_employee_with_lump_sum_over_max.rb - is this a module? Is this a separate class? How can I pull these out and then extend my ScenarioEntry based on these conditions?

(Ben Orenstein) #2

If you’re reading Ruby Science, have you checked out the section on Extract Class? It covers the mechanics and reasoning behind pulling out new classes quite well.

Is there something in that section that’s not clear that I can clarify?

(Jarrett Green) #3

Thanks for the reply. I was getting nervous :wink: I don’t think I’ve gotten there. I got maybe 20 pages in and my brain started whirring. I’ll take a look.

(Jarrett Green) #4

So I can see how I could maybe create a bunch of classes for each condition we might need to deal with in the budget scenario and then use an set up something like:

@type_of_scenario_entry = WhateverTheClassThatHasTheMethodsWeNeed.new

And pass that to the methods in my main class ScenarioEntry Class?

(Geoff Harcourt) #5

Getting started with moving to small, single-responsibility objects is hard. My advice is not to try to make it perfect immediately, but to start doing small, incremental things to make it better. If you have tests that cover your existing class, try extracting just one class that does a specific thing, and leave the rest of the mess intact. Then just keep doing it piece by piece.

I used to freak out when I looked at big, messy classes, thinking they always had to be rewritten from scratch. If the currently work but are just hard to refactor (because of their size and messiness), that’s often a good candidate for your initial efforts at learning to build smaller, more sharply-focused classes.

After the experience of fixing a big mess, you’ll have the advantage when you are writing new classes. A common pressure I use is that if writing the tests (especially the setup) is really hard, then your object is almost certainly doing too much.