ASCIIcasts

10: Refactoring User Name Part 1 

(view original Railscast)

Other translations:

In this episode we’re going to discuss refactoring. Refactoring is improving the design of code without changing its functionality. This can be done to remove duplication or improve readability or maintainability.

In this application we have a list of users. Clicking on a user shows their profile. Notice that some users have a middle initial, while others don’t.

The index and show views in the browser.

Let’s take a look at the view code. First, the view code for the index:

<h1>Users</h1>
<ul>
<% for user in @users %>
<li>
  <a href="<%= user_path(user) %>">
    <%= user.first_name %>
    <%= "#{user.middle_initial}." unless user.middle_initial.nil? %>
    <%= user.last_name %>
  </a>
</li>
<% end %>
</ul>

The index view code

The code above gets all of the users, then loops through them. The three lines within the <a> element display the user’s name. Now let’s see the profile page.

<h1>Profile</h1>
<p>
  Name:
  <%= @user.first_name %>
  <%= "#{@user.middle_initial}." unless @user.middle_initial.nil? %>
  <%= @user.last_name %>
</p>
<%= link_to 'Users List', users_path %>    

The show action’s view code

The profile page has the same three lines of code to display a user’s name as the index page. This is obvious duplcation and a good candidate for refactoring. The code to display the user’s name splits the name up into three parts. This is done so that the middle initial can be shown with a full stop after it, unless the user has no middle initial.

How do we begin refactoring?

One way to refactor this code would be to move it into a helper method, but there is no HTML in the code, just the user’s name, which is related to the User model. The ideal place to put this repeated code would therefore be in the user model. Let’s create a method in the user model called full_name. We’ll start by moving the repeated code from the index and show views, put the three parts of the name into a local variable, then return the local variable.

class User < ActiveRecord::Base
  def full_name
    name = first_name + ' '
    name += "#{middle_initial}. " unless middle_initial.nil?
    name += last_name 
    name
  end
end

The User model after we’ve added the full_name method.

As we’ve moved the code into the user model the @user variable can’t be referenced. But, as we’re calling methods in the User model so we can simply remove it.

Does our refactor work?

Now that we’ve written our full_name method we can replace the code in our index and show methods. Let’s look at the relevant part of the index view.

<a href="<%= user_path(user) %>">
  <%= user.full_name %>
</a>

The index view with the full_name method.

There’s another way we can improve the code above. Because we had such complex code to generate the user name we had to use a standard HTML <a> tag, rather than link_to. Now, we can use link_to and simpify the view code even further.

<%= link_to user.full_name, user_path(user) %>

The index view with link_to.

The code in the User model and its views is now much improved from where we started. We’ve removed duplication and moved user-related code that belongs in the model in to the User model. There’s still room for improvement though, and we’ll see how the code can be improved further in the next episode.

The code after refactoring

class User < ActiveRecord::Base
  def full_name
    name = first_name + ' '
      name += "#{middle_initial}. " unless middle_initial.nil?
      name += last_name 
      name
  end
end

The User model after refactoring.

<h1>Users</h1>
<ul>
<% for user in @users %>
    <li><%= link_to user.full_name %></li>
<% end %>
</ul>

The index view after refactoring.

<h1>Profile</h1>
<p>Name: <%= @user.full_name %></p>
<%= link_to 'Users List', users_path %>

The show view after refactoring.