CODEWITHSUNDEEP
ruby-on-railsactiverecordrefactoring
msaspence
msaspence

Reputation: 1424

Refactoring Fat ActiveRecord Model, What Now?

I've been using CodeClimate to improve my code base and I have a model class that is falling down on 'definition outside of methods' and 'total overall complexity' for those that don't use CodeClimate the definition outside of methods refers to class definitions like associations and validations. And total overall complexity is what it says, ie non of my individual methods are too complex to overall the class is to complex.

Now I appreciate this could be a false position, but that D class on my score is annoying and there may well be more I can do to improve this class, so my question is:

What if anything should I do to simplify this ActiveRecord class?

My main problem is I've exhausted all the written about ways to refactor a fat model, using this article http://blog.codeclimate.com/blog/2012/10/17/7-ways-to-decompose-fat-activerecord-models/ and similar, I am basically left with:

Its a closed source project, but given the lack of domain logic in this class I've shared it here. https://gist.github.com/msaspence/5317419

Upvotes: 3

Views: 1278

Answers (1)

apneadiving
apneadiving

Reputation: 115541

Callbacks are hell. I mean, they are useful but they quickly overload your models.

Do you have an action in your controller to handle the unpublish thing?

If so (which I hope), this could be moved / extracted to a service object:

after_save :email_author_if_first_published
def email_author_if_first_published
  if pending_was == true && pending == false
  self.create_activity key: 'action.publish'
  delay.send_action_published_email
end
end

def send_action_published_email
  ActionAuthorMailer.action_published(self, self.user).deliver
end

This should not belong to the model, some presenter should be better:

def mixpanel_data_points
  {
    campaign_id: campaign_id,
    cause_id: cause_id
  }
end

def open_graph_type
  'campaign_action'
end

def facebook_invite_message
  "Please help me raise awareness on PatientsCreate. It'll take you 5 seconds and could create genuine change. Thanks"
end

def percentage_completed
  v = (participation_count.to_f/participation_target_count.to_f)*100
  v > 100.0 ? 100.0 : v
end

This seems not good:

after_update :update_campaing_actions_count
def update_campaing_actions_count
  Campaign.decrement_counter(:actions_count, self.campaign_id_was)
  Campaign.increment_counter(:actions_count, self.campaign_id)
end

Because you do this even if campaign_id was not changed

This looks bad:

def url
  open_graph_url "/actions/#{id}"
end

Can't tell much more, I don;t really know what is sheer data or presentation in the other methods

Upvotes: 3

Related Questions