CODEWITHSUNDEEP
ruby-on-railscallbackmodelsobservers
Squadrons
Squadrons

Reputation: 2557

Rails callbacks, observers, models, and where to put methods and logic

I'm working on an app at work. Basic stuff, user signs up (with an associated organization).

Initially I started off with a simple controller - 

# Need to check if organization exists already; deny user creation if it does 
if @organization.save
  @user.save
  redirect_to user_dashboard_path...

I soon found myself in a callback soup:

After the organization is validated, we save the user. When the organization is created, I create another two models, EmailTemplate and PassTemplate (an organization has_one :email_template, has_one :pass_template)

  after_create :init_company, :init_email_template, :init_pass_template, :init_form

Each of those callbacks generally calls method on the model, something like: 

def init_email_template
  self.email_template.create_with_defaults
end

Initially I thought this was quite clever - doing so much behind the scenes, but I've been reading Code Complete by Steve McConnell, and feel this is not simple at all. If I didn't know what was happening already, There's no hint that any time an organization is created it creates 3 associated objects (and some of those objects in turn initialize children objects).

It seems like a bad programming practice, as it obfuscates what's going on.

I thought about moving all of those initalizations to the controller, as an organization is only ever created once:

class OrganizationsController < AC
  ...
  def create
    if @organization.save
       @organization.create_user
       @organization.create_email_template
       @organization.create_pass_template
  end

That seems like cleaner code, and much easier to follow.

Question 1 *Are there better solutions, or best practices for handling creating associated objects upon creation of the hub object that I'm unaware of?*

Side note - I would have to rewrite a bunch of tests that assume that associations are automatically created via callbacks - I'm okay with that if it's better, easier to understand code.

Question 2 **What about a similar situation with after_save callbacks?**

I have a customer model that checks to see if it has an associated user_account after creation, and if not, creates it. It also creates a Tag model for that user_account once we've created the user_account

class Customer < AR
  after_create :find_or_create_user_account

  def find_or_create_user_account
     if !self.user_account_exists?
        #create the user
     end
     Tag.create(:user_id => self.user_account.id)         
  end
end

Somewhat simplified, but again, I believe it's not particularly good programming. For one, I'm putting logic to create two different models in a third model. Seems sloppy and again the principle of separating logic. Secondly, the method name does not fully describe what it's doing. Perhaps find_or_create_user_account_and_tag would be a better name, but it also goes against the principle of having the method do one thing- keeping it simple.

After reading about observers and services, my world was thrown for a bit of a loop.

A few months ago I put everything in controllers. It was impossible to test well (which was fine because I didn't test). Now I have skinny controllers, but my models are obese and, I think, unhealthy (not clear, not obvious, harder to read and decipher for another programmer/myself in a few months).

Overall I'm just wondering if there are some good guides, information, or best practices on separation of logic, avoiding callback soup, and where to different sorts of code

Upvotes: 0

Views: 452

Answers (1)

Rystraum
Rystraum

Reputation: 2025

Why not the following?

after_create :init_associated_objects

def init_associated_objects
  init_company
  init_email_template
  init_pass_template
  init_form
end

My interpretation with "a method should do one thing" isn't strict and that I usually have a method that calls other method (much like the one above). At the end of the day, it's a divide and conquer strategy.

Sometimes I create utility POROs (plain old ruby objects) when it doesn't make sense to have an AR model but a group of functionalities is a class' responsibility. Reports, for instance, are not AR-backed models but it's easier when a report that needs to call multiple models is just instantiated once where the reporting period start and end are instance variables.

A rule of thumb that I follow: if I instantiate the models outside of the whole MVC stack (e.g. Rails console), the things that I expect to happen should stay inside the model.

I don't claim best practices but these have worked for me so far. I'm sure other people would have a better idea on this.

Upvotes: 1

Related Questions