Spring cleanup for object creation in Rails

TL;DR A complex object creation can clutter a controller. It’s better to move it into a dedicated method of the corresponding model.

Introduction

Weeds

Cut your Weeds asap.

As software grows, what was once good solution can grow to become a small nuisance – like inoffensive weeds at the side of your driveway. Leaving those weeds unchecked, they become a festering plague. Time to act.

Today, I’ll cover one of those cases in Rails: the creation of a new resource.

My goal into this post is to underline the responsibility of the controller and to not clutter it with creation concern. I will offer a first simple solution that allows to fix it.

The Set-Up

The Blog application as the one illustrating the Rails guide. Nothing is complex in the beginning, and the Post controller looks like a scaffold.

class PostController < ApplicationController
 #...
 def create
   @post = Post.new(params[:post])
   if @post.save
     redirect_to @post
   else
     render action: 'new'
   end
 end
 #...
end

Nothing exceptional.

A Few Weeks Later…

The Blog application has grown: to each post, some statistics are attached such as the number of words, and the used language is automatically detected.

class PostController < ApplicationController
 #...
 def create
   @post = Post.new(params[:post])
   @post.number_of_links = count_links(@post.body)
   @post.number_of_chars = count_chars(@post.body)
   @post.number_of_words = count_words(@post.body)
   @post.language = detect_language(@post.body)
   if @post.save
     redirect_to @post
   else
     render action: 'new'
   end
 end
 #...
 private
 def count_links(text)
   # …
 end
 def count_chars(text)
   # …
 end
 def count_words(text)
   # …
 end
 def detect_language(text)
   # …
 end
 #...
end

This code wasn’t produced in one change. It’s the result of several changes – made at different moments. It looked like the natural place to add everything concerning the creation of a new post. One more line didn’t seem to be a problem. Now there are 4 – without counting all the lines for the private methods – 4 more reasons to change the code when creating a new post.

The responsibility of your controller is to make sense of the request and to produce the appropriate output (as well expressed in the Rails Guides) – not to take care of the dirty details of a post creation.

The Cut – The Cleanup

A better place to put those details is the Post model. You could, for instance, use one of the Rails callbacks called when creating an object. Even if it is a totally valid option, I don’t use it – as it depends on the Rails framework. I prefer to have a model that could live without it – it helps to have a better OO design and faster tests (Avdi Grimm’s book Objects on Rails gives a great overview of that approach).

The idea is to put all the creation details into a same class method that will return an instance ready to be saved:

class Post < ActiveRecord::Base
 #...
 def self.build(params)
   post = Post.new(params)
   post.number_of_links = count_links(post.body)
   post.number_of_chars = count_chars(post.body)
   post.number_of_words = count_words(post.body)
   post.language = detect_language(post.body)
   post
 end
 #...
 private
 def self.count_links(text)
   # …
 end
 def self.count_chars(text)
   # …
 end
 def self.count_words(text)
   # …
 end
 def self.detect_language(text)
   # …
 end
end

 The Post controller is much simpler now:

class PostController < ApplicationController
 #...
 def create
   @post = Post.build(params[:post])
   if @post.save
     redirect_to @post
   else
     render action: 'new'
   end
 end
 #...
end

Now, everything about the creation of a Post instance is in one dedicated class method of the model. After all, it’s totally the responsibility of the model. This is commonly called a factory method. There is a very common and known method that already does that: the constructor, i.e. the method initialize. Outside of Rails, it’s better to put it in the constructor (except for some specific reasons, e.g. giving it a specific name). But this is Rails – and overriding the constructor will get rid of the ActiveRecords magics (yet it’s still possible by explicitly reproducing the magics  but don’t do this at home).

This is the first step to the Factory Method Pattern . It’s goel is is to isolate complex process of creation from the object itself – especially when it goes beyond its responsibility or could result into different types. As this is  not (yet) our case, I won’t go develop further on this.

Having said that, we can finish our cleanup by refactoring our private methods. As we are now into the Post model, we can change them into object method.

class Post < ActiveRecord::Base
 #...
 def self.build(params)
   post = Post.new(params)
   post.calculate_the_text_stats
   post.detect_language
   post
 end
 #...
 private
 def calculate_the_text_stats
   # counting chars, words, and links
   # setting the corresponding attributes
 end
 def detect_language
   # detecting the language and setting the eponym attribute
 end
end

References

Some references if you want go beyond.

Conclusion

Rail track

Don’t let your Rails overwhelmed by weeds.

When creation becomes complex, it clutters the controller with details that are not its responsibility. It’s necessary to move it elsewhere. We’ve elaborated on one possible solution: dedicating a class method of the model to that complex creation. That method is commonly called a factory method (but it’s not a complete implementation of the Factory Method Pattern).

Don’t be overwhelmed by the weeds on your coding driveway. Ask a reviewer to have a look at your code, or use a tool such as RailsBestPractices to automatically detect it.

Enhanced by Zemanta




15 thoughts on “Spring cleanup for object creation in Rails

  1. Mario Olivio Flores

    Instead of defining Post.build, why not add before_save :calculate_the_text_stats, :detect_language (or before create). Or add an observer if you prefer that approach instead of the filter. Then in your controller action you don’t even need that first line and could just do if Post.create!.

    Reply
    1. toch Post author

      Hi Mario,

      I agree that using callbacks in the controller will work. But as I said in my post,

      I prefer to have a model that could live without it – it helps to have a better OO design and faster tests

      IMO, this is indeed more the responsibility of the model than of the controller. I would use callbacks or observers to triggers actions and changes that goes beyond the scope of the Post instance, for instance checking the permission of the user to do it or sending an email to someone.

      Mario, except to do Post.create!, why do you think callbacks and observers are better options?

      Reply
      1. Mario Olivio Flores

        Hello,

        Kris is right, I was referring to callbacks on the model. before_save is a ActiveRecord callback, my mistake for referring to it as a filter later in my comment. I can see how that was confusing.

        Generally, I think it’s a bit dryer if you can chop the code in your controller down to just if Post.create! and completely replace your 6 line Post.build method with a one liner before_save. Additionally, if you update your Post instance, you’d always have to manually call calculate_the_text_stats and detect_language, but taking advantage of a model callback takes care of this for you.

        Make sense?

        -Mario

        Reply
  2. James

    I don’t agree with moving that function to the model, but I do agree that the controller was the wrong place for it.

    I would suggest create a ruby class called PostsManager and start collecting these post related actions as method in this class. The @post.save logical action should also be performed inside the …Manager class.

    The path your on seems to be imbedding your business logic inside the Rails framework via any means necessary. I’m hoping you don’t stay on that path.

    Reply
    1. toch Post author

      Hi James,

      In fact, I totally agree :). I would also go to a full implementation of a Factory Method Pattern. As you write, it’s important to separate business logic from the framework.

      I’ve chosen to present the solution of simple dedicated methods into the model for two reasons:

      1/ When refactoring, I like to go step by step. It’s true that a Factory Method Pattern is more flexible, but if the creation isn’t so complex, I’d say it’s overkilled: it’s a lot of code in case of a possible future need of flexibility that could never happen. What’s your opinion on that point?

      2/ As a former teacher, I’ve learnt it’s also important to decouple the concepts. My goal into that post was to underline the responsibility of the controller. Then, I wanted to offer a first simple solution that allows to fix a common mistake with creation put into the controller. It seems I wasn’t clear enough. Do you think I should state it clearly at the beginning?

      Anyway, thanks Jame for your comment.

      Reply
      1. James

        1. If the discussion is anchored on Rails then I have shy’ed away from factory patterns, as I have yet to find a good use case for them in the lifecycle of requests/responses in Rails. Most objects that could be useful in the Rails Controller environment are usually simple and short lived. Their short life span being one of two very important factors that I consider. Since everything created during a request cycle is garbaged collected after the response is sent, I tend to focus on extracting the maximum use out of each objected create, while trying to adhere to SOLID design techniques. However, the design notion of Presenters and Decorators does influence my approach. The net is I prefer create objects that have a strong relationship to what could be called a Domain object.

        2. Restated that way, with some hint that architectural goals should have influence on the solution would be as helpful, as your original statement.

        Reply
  3. James


    # /app/services/posts_manager.rb
    class PostsManager
    attr_accessor :user, :ui
    def initialize(params)
    @user = params[:user]
    @ui = params[:ui]
    end
    def create_method(params)
    # builds a complete record
    # creates and save a Post as new_record
    # returns a hash of
    {success: save_rtn, record: new_record, message: "any flash message"}
    end
    # ... other methods ...
    end

    # /app/helpers/application_helper.rb
    module ApplicationHelper
    def posts_manager
    @caching_post_manager ||= PostsManager.new({user: current_user, ui: self})
    end
    end

    # /app/controllers/post_controller.rb
    class PostController < ApplicationController
    #...
    def create
    @page_controls = posts_manager.create_method(params)
    @post = @page_controls[:record]
    if @page_controls.sucess
    redirect_to @post
    else
    render action: 'new'
    end
    end

    Would actually like to have views changes to work with @page_controls, rather than hundreds of @vars; one nice and neat container.

    Reply
    1. toch Post author

      Great snippet! Thanks James.

      I’d prefer to call it PostFactory and have methods create and update. I think the name PostManager could be misleading and start to be the big class where to put everything that helps to manage a Post.

      May I reuse a part of your code for an update of the post?

      Reply
      1. James

        Feel free to use any portion of my posts without reservation.

        PostFactory, PostsManager, etc. I don’t know what the proper domain object’s name would be that encompass data objects like Posts, and Comments; but that’s the name I would give it – maybe CommunicationDomain and posts and comments would be one channel of communications internal to them or just simple data entities.

        My intent of the snippet was to remove from the controller and view any knowledge of the data model and restrict the controller to its original role of accepting a requests and returning a proper response. Controllers are likely our sole interface to the outside and the user

        If you accept the notion of Domain (or Service) objects being the entity that services the request and prepares all data needed for a valid response. Then I’ve chosen to anchor my service/domain objects to the ApplicationController via those cached helpers. As the app grows and more than one domain/service object exists having them cached will improves performance. Note they cannot or should not be serialized as objects. It is foreseen that one service object would call another service object using a controller helper to gain a reference to the other service object.

        I used a hash as the return value of the service object’s method, I would typically standardize the elements of the hash to contain elements for Controller operations and the data for the expected view. overall this reduces the total number of @vars in the controller/view memory space and I hope make the result easier to maintain and read.

        Domain/Service objects (searching for the right name) do contain the ActiveModel/ActiveRecord model IO statements. But they are wrapped in methods inside the service object to facilitate testing and replacement. Having all of ActiveRecord’s model statement in one place and wrapped with simple methods has save me a lot time when upgrades and or implementation changes are needed.

        If you don’t like the @page_controls[:key] notation or just prefer an object dot notation. I have a ResultBean class the provides object “.” notation and “?” present predicates, which I find helpful inside of views. It will be in a shell project I’m preparing for github and will publish in a few days; otherwise email me and I will send a zipped version with all the rspec tests.

        I hope I’ve conveyed enough to make my contribution helpful.

        Cheers,

        Reply
        1. toch Post author

          Thanks James for the very clear explanations. It will be very helpful as I will write a sequel based on the different feedback.

          About your shell project, I’m curious. Tell me when it’s published on GitHub.

          Reply
  4. James

    Christophe,
    Let me apologize for the tone of my prior post, I mean no disrespect and actually appreciated your article. It correctly pointed out one of the critical code quality concerns we face daily, and offered a viable solution. Maybe like you, I am evolving a strategy to address this problem, which I partially shared in code.

    I’ve had the privilege to mentor several Rails newbies lately and they consistently show me articles and google findings that solve this problem by moving it into some other MVC component of Rails; which is no more than kicking the can down the road.

    Thanks again for sharing.

    Reply
    1. toch Post author

      James, no apologizes needed. It’s me that answer you so late.

      I’ve had the privilege to mentor several Rails newbies lately and they consistently show me articles and google findings that solve this problem by moving it into some other MVC component of Rails; which is no more than kicking the can down the road.
      I couldn’t agree more, and great quote by the way :).

      Thanks again for your comments, don’t hesitate in the future, it’s exactly the kind of discussion I hope to initiate by publishing a post.

      Reply

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>