CODEWITHSUNDEEP
ruby-on-railsrubyrefactoring
Piotr Kruczek
Piotr Kruczek

Reputation: 2390

Best practice for fat model refactor

I'm trying to make my fat User model less clunky. I'm using value objects to represent custom values and operations on them, and am stuck with ActiveSupport::Concerns and modules. I read this as an inspiration.

I put helper methods like this:

def is_a_wizard?
  power_level >= WIZARD_POWER
end

def just_became_a_wizard?
  power_level == WIZARD_POWER
end

into modules, and included them as sort of extensions. However, it is hard to read and maintain, and I need some of them both in views and controllers (for example for wizard authentication). Where should I put them? Create service objects for when they're used?

Upvotes: 5

Views: 415

Answers (4)

hedgesky
hedgesky

Reputation: 3311

You can create additional class and use it wherever you want:

# lib/wizard_detector.rb
class WizardDetector
  def initialize(power_level)
    @power_level = power_level
  end

  def is_a_wizard?
    @power_level >= WIZARD_POWER
  end

  def just_became_a_wizard?
    @power_level == WIZARD_POWER
  end 
end

# app/models/user.rb
class User

  delegate :is_a_wizard?, :just_became_a_wizard?, to: :wizard_detector

  def wizard_detector
    @wizard_detector ||= WizardDetector.new(power_level)
  end
end

# anywhere else
WizardDetector.new(power_level_to_check).is_a_wizard?

Please notice wizard_detector object is cached in model, maybe it is harmful if power level changes during request flow. It's ok to replace caching.

Upvotes: 2

yarnosh
yarnosh

Reputation: 74

I've started using Concerns more and more lately to keep models cleaner. I would create a module for "Wizard" with the Concern modules under that namespace for models and controllers. Though I'd personally be a little suspicious if you had that many controller methods related to wizards. For views, why not just write standard Helpers?

This makes more sense, of course, if you have more than one model that needs the methods, but it can still help clean up single models. I don't personally find it difficult to maintain as I hardly ever touch these little methods. You may also find along the way that you can move more than method definitions to a Concern.

Also, you should make sure your Concerns are actually concerns and not just a generic dumping ground for methods you're tired of looking at in your model.

Upvotes: 0

floum
floum

Reputation: 1159

I would rename the methods in your example :

#is_a_wizard? would be #wizard? (this one would be the ruby way)

just_became_a_wizard? => new_wizard? (inexperienced_wizard?... This one is less obvious)

There are only two hard things in Computer Science: cache invalidation and naming things.

-- Phil Karlton

Upvotes: 0

apneadiving
apneadiving

Reputation: 115541

As I wrote in comments, business logic methods are ok in the model.

But if you feel like its too much, you could think about something like

class User < AR::Base

  def predicates
    @predicates ||= ::User::Predicates.new(self)
  end

end

and in /models/user/predicates.rb

class User
  class Predicates < SimpleDelegator
    def just_became_a_wizard?
      power_level == User::WIZARD_POWER
    end
  end
end

Then you could do:

 user.predicates.just_became_a_wizard?

The huge benefit is that your user model isnt cluttered with tons of methods anymore.

The huge drawback is you need to call the proxy object each time.

Upvotes: 0

Related Questions