Topic: Refactoring on Rails: Move to Model

This is the first article in a series which will cover various refactoring techniques that are specific to Rails. Refactoring is used to improve the design of existing code without changing its functionality.

The first refactoring I will be discussing involves taking a bit of logic out of the view and placing it in the model. This can clean up the views quite nicely.

Let's start out with a very simple example. We have a Person model with first_name, last_name, and middle_initial attributes. In our view we have this code:

Full Name:
<%= @person.first_name %>
<% unless @person.middle_initial.blank? %>
  <%= @person.middle_initial %>.
<% end %>
<%= @person.last_name %>

Not very pretty. Even worse, this code is repeated several times throughout the application. To remove this duplication and to clean up the views, we can define a method in the Person class to handle this logic.

class Person < ActiveRecord::Base
  def full_name
    result = first_name + ' '
    unless middle_initial.blank?
      result += middle_initial + '. '
    end
    result += last_name
    result
  end
end

This makes the view much nicer:

Full Name: <%= person.full_name %>

We successfully shoved the logic into the Person model, but I'm still not satisfied. I think we can clean up the full_name method, but before we do that, let's create some tests to make sure everything continues to work as we do the cleaning. Tests are very important when refactoring to make sure we don't break anything as we improve the code.

# in person_test.rb
def setup
  @person = Person.new(:first_name => 'John', :last_name => 'Smith')
end

def test_full_name
  assert_equal 'John Smith', @person.full_name
end

def test_full_name_with_middle_initial
  @person.middle_initial = 'A'
  assert_equal 'John A. Smith', @person.full_name
end


Good, the tests are passing so now it's time to improve the full_name method. I would like to remove that temporary "result" variable and merge it all onto one line. I can use an array to help me out. Maybe something like this:

def full_name
  [first_name, middle_initial, last_name].join(' ')
end

That looks nice and tidy, but unsurprisingly it fails both of the tests. It fails the second test because there's no period, so let's create a new method to return a middle initial with a period:

def middle_initial_with_period
  middle_initial + '.' unless middle_initial.blank?
end

def full_name
  [first_name, middle_initial_with_period, last_name].join(' ')
end


The second test is working now, but the first test still fails because there's an extra space in there when no middle initial is defined. We can remove that by using array's "compact" method like this:

def full_name
  [first_name, middle_initial_with_period, last_name].compact.join(' ')
end

Our tests are all passing now. Yay!

Wait one minute, I hear someone say, the compact method just removes nil values, what if the middle initial is an empty string? Well let's add a test and find out:

def test_full_name_with_empty_middle_initial
  @person.middle_initial = ''
  assert_equal 'John Smith', @person.full_name
end

Surprisingly this passes successfully without us changing anything. Why? Because the middle_initial_with_period method already takes care of empty strings and will return nothing (nil) if middle_initial is blank.

In this tutorial you learned how to refactor code from the view into the model, but this can just as easily be applied to code in the controller or any other part of the application. Your job is to look through your current rails projects for code that can be moved into a model. Usually if it can be moved into a model, it should, but just be careful to keep view/controller specific code out of the model. For example, don't move HTML code into the model.

If you found this useful, keep an eye out for the next Refactoring on Rails article. In the meantime, check out Martin Fowler's excellent book: Refactoring.

Check out the next article in this series: Multiple Scopes in Controller

Last edited by ryanb (2006-11-16 12:29:48)

Railscasts - Free Ruby on Rails Screencasts

Re: Refactoring on Rails: Move to Model

Nicely done! I wish I'd seen this before I wrote my online billing system. I did end up doing this, but it was a lot more muddling through that I'd have otherwise had. wink

Re: Refactoring on Rails: Move to Model

RAILSforum is turning out to be a great resource for Tutorials.
Nice work 

Added this one as well to my growing (140+) collection below

Last edited by boyles (2006-09-25 18:07:55)

Re: Refactoring on Rails: Move to Model

Excellent tutorial, thank you Ryan.  I was starting to wonder what the Model's were for. wink

I should really get a rails book...

Re: Refactoring on Rails: Move to Model

This refactoring is great and I'm looking forward to the rest of this series of tutorials.

I've done something similar in my person model to create a full_address method from all the individual address fields. I'm sure this could be DRYied up somewhat.

    def full_address
        result = ''
        unless address_line_1.blank?
            result += address_line_1
        end
        unless address_line_2.blank?
            if result == ''
                result += address_line_2
            else
                result += ', ' + address_line_2
            end
        end
        unless address_line_3.blank?
            if result == ''
                result += address_line_3
            else
                result += ', ' + address_line_3
            end
        end
        unless address_line_4.blank?
            if result == ''
                result += address_line_4
            else
                result += ', ' + address_line_4
            end
        end
        unless town.blank?
            if result == ''
                result += town
            else
                result += ', ' + town
            end
        end
        unless county.blank?
            if result == ''
                result += county
            else
                result += ', ' + county
            end
        end
        unless postcode.blank?
            if result == ''
                result += postcode
            else
                result += ', ' + postcode
            end
        end
        result
    end

The above puts whatever parts of the address that exist all on one line with commas after each bit. However in a different view on my app I want to display the address differently with each part on a separate line with <br> in between instead of commas. Would I be best creating a new method full_address_multi_line or can I adapt my existing method to do both?

Also when I am displaying the person's profile details in the view, where a field is not completed I might want to display "Not supplied" or "N/A". Is this best handled in the view or could it be moved to the model too? At the minute I've got a lot of stuff like this in my view:

<%= if @person.job_title.blank? 
            "Not supplied"
        else
            @person.job_title
        end %>

Re: Refactoring on Rails: Move to Model

Very good questions Cathy!

The full_address method can be greatly simplified by using a technique similar to what I did in the article - which is placing the address sections in an array and joining that.

def full_address(join_str = ', ')
  sections = [address_line_1, address_line_2, address_line_3] # etc...
  sections.delete_if(&:blank?) # removes any blank entries from the array
  sections.join(join_str)
end

Notice I passed the join string as a parameter, this way you can change it when calling the method.

You can go one step further to clean this up a bit more:

def full_address(join_str = ', ')
  address_sections.delete_if(&:blank?).join(join_str)
end

def address_sections
  [address_line_1, address_line_2, address_line_3]
end


As for your second question, I would hesitate to move this "Not Supplied" logic into the model. First of all you will need to handle this for many different attributes and models. Second, if you override the original attribute it will probably cause problems at the model/controller level. Lastly you may want to handle this differently in different views.

Certainly keeping this in the view isn't optimal either. What I would do is create a helper method that handles this for you. You can then input any value you want and it will return "Not Supplied" automatically when blank. For example:

# in view
<%= not_supplied_if_blank @person.job_title %>

# in application_helper
def not_supplied_if_blank(value)
  value.blank? ? "Not Supplied" : value
end


You may want to rename that method.

Last edited by ryanb (2006-11-15 14:51:41)

Railscasts - Free Ruby on Rails Screencasts

Re: Refactoring on Rails: Move to Model

Thanks Ryan for the lovely code - you've certainly made my full_address method a lot more elegant and useful. cool

After I had posted I found some examples online of using application helpers and ended up writing pretty much what you wrote for showing "Not supplied" when something was blank. I've not really used helpers a great deal yet but am starting to. I guess when things get repetative in the view and don't seem to fit in the model then helpers are a good solution.

Both my view and model are now looking a lot better. Thanks again for the great advice. BTW I think you should link to the other refactoring tutorials at the top of this one.

Re: Refactoring on Rails: Move to Model

Thanks for this post - it's really helpful while learning ruby/rails.

Regarding the following:

sections.delete_if(&:blank?)

I understand what it does but haven't been able to find any information on the &: part. Could you please explain what this means or send a link to a doc explaining it?

Last edited by lassebunk (2009-05-03 15:19:39)

Re: Refactoring on Rails: Move to Model

http://blog.hasmanythrough.com/2006/3/7 … -shorthand

google for "rails symbol to proc" for more blog posts etc. about this

Re: Refactoring on Rails: Move to Model

Regarding the following:

sections.delete_if(&:blank?)

Although it looks cool, it is not recommended to use that kind of shortcut.

Re: Refactoring on Rails: Move to Model

Duplex wrote:

http://blog.hasmanythrough.com/2006/3/7 … -shorthand

google for "rails symbol to proc" for more blog posts etc. about this

Johnson wrote:

Although it looks cool, it is not recommended to use that kind of shortcut.

Thanks for your replies - I'll look into it... And not use it after reading Johnson's reply ;-)

Re: Refactoring on Rails: Move to Model

lassebunk wrote:

Thanks for your replies - I'll look into it... And not use it after reading Johnson's reply ;-)

I agree the symbol-to-proc syntax looks somewhat confusing but once you get used to it I find it a useful shorthand and would not avoid using it.  In fact while it started as a Rails extension to the Ruby class Symbol it has moved into the Ruby core with 1.8.7 and 1.9 so I expect we'll all be seeing more of it.

Several months ago I got curious about how it all worked and wrote up a post of my investigations which you may find useful http://www.alexrothenberg.com/2009/02/i … works.html and of course Ryan Bates did a great Railscast of the same several months after he posted this article which you can watch at http://railscasts.com/episodes/6-shortc … ol-to-proc

Re: Refactoring on Rails: Move to Model

alexrothenberg wrote:

Several months ago I got curious about how it all worked and wrote up a post of my investigations which you may find useful http://www.alexrothenberg.com/2009/02/i … works.html and of course Ryan Bates did a great Railscast of the same several months after he posted this article which you can watch at http://railscasts.com/episodes/6-shortc … ol-to-proc

Thanks for the clarification, Alex. Much appreciated.