[ToDoList] Refactoring (Tidying Up)

We can be proud of ourselves; we have created a neat little app that utilises both the MVC and CRUD principles of web development and helps us keep abreast of all our little jobs. Seriously, well done!

However, as we alluded to at several points during the last few sections, we have certain methods in our controllers that duplicate each other and some HTML in unideal locations. Again, this was fine while we were actively developing our application, however we should always put just as much effort into tidying up our code and making it as optimal as possible. This is often referred as making your code DRY: Don't Repeat Yourself.
Enter the refactoring stage!

1. Introducting Partials
2. Partial to a bit more Refactoring
3. Tidy Controllers Are Happy Controllers
4. Quite a Title

 

---

 

You may remember that earlier we added a charming site heading & navigation menu, and a nice confirmation message using flash, into our view template (/app/views/layouts/application.html.erb). We should always try and avoid adding HTML to this template where possible for the sake of elegance, and instead import these little bits from separate files called partials.

As the name suggests, partials are partial sections of HTML that import into the main page template, very similarly to the yield tag, and are denoted by an underscore (_) prefix in the filename.

Let's move our site heading & navigation bar into its own partial just now by cutting our nav bar out of the view template and pasting it into a new file: /app/views/layouts/_navigation.html.erb:

<div align="center">
  <h1> ToDoList </h1>
  <h4>
    <%= link_to 'Home', root_path %>
    | <%= link_to 'My To-Do List', todos_path %> <!-- again using route prefix and _path -->
    | <%= link_to 'About', about_path %>
    | <%= link_to 'Help', help_path %>
  </h4>
  <hr />
</div>

Once you've done this, we can reference this new file from the template by adding the following to where the nav bar was previously in the template file at /app/views/layouts/application.html.erb:

    <%= render 'layouts/navigation' %>          <!-- don't have to include _ prefix here -->

Refresh your browser page, and you will see the nav bar and site heading exactly where you left it. However if you later need to edit this navigation for whatever reason, it will be a lot easier to find and read from its own partial!

Let's do the same with the flash messages we added to confirm if form submission was successful - cut the flash block from the template at application.html.erb and paste into a new file: /app/views/layouts/_messages.html.erb:

<div>
  <% flash.each do |k, msg| %>          <!-- iterating a Hash, so specify key and val vars -->
    <ul>
      <li><%= msg %></li>
    </ul>
  <% end %>
</div>

Now reference this again from where this was in the template in the same way as before:

    <%= render 'layouts/messages' %>           <!-- no need to specify file type either! -->

And once again refresh your browser, create a new To-Do item and see the same functionality, only much nicer to deal with as a developer!

 

---

 

If we look at the new and edit forms we can see that both the forms used, as well as the error display, are identical to one another.
Ew.
Let's cut these out of one of the files and paste it into a new partial. However, because the only files that will use it are in the todos directory, we can simply put the file for new partial in here: /app/views/todos/_form.html.erb.

<% if @todo.errors.any? %>                             <!-- check if any errors present -->
  <h3> Your To-Do item could not be saved: </h3>      <!-- if yes, display the messages -->
  <ul>
    <% @todo.errors.full_messages.each do |msg| %>      <!-- for all @todo err messages -->
      <li> <%= msg %> </li>                                <!-- display message in list -->
    <% end %>
  </ul>
<% end %>

<%= form_with model: @todo do |td_form| %>    <!-- using '=' erb tag as we're inserting -->

  <div>
    <%= td_form.label       :name %>             <!-- specifying form field & its value -->
    <br />
    <%= td_form.text_field  :name %>
  </div>

  <div>
    <%= td_form.label     :description %>               <!-- label of description field -->
    <br />
    <%= td_form.text_area :description %>                <!-- area to enter description -->
  </div>

  <div>
    <%= td_form.submit %>                          <!-- submit the data to @td variable -->
  </div>

<% end %>                                                       <!-- end the Ruby block -->

As before, we can now remove this code from each view that uses it (new and edit) and replace it with a reference to our new partial. Note that, unlike when we partialised our nav bar and flash messages, we do not need to specify the full relative path of the partial because it is in the same directory as the views (not templates) that are being served:

  <%= render 'form' %>                                       <!-- no path necessary here -->

Refresh your browser once again, create a new To-Do item and see your now elegantly handled form! For the sake of completionism, click its Edit link and see it there too!

 

---

 

We can use this same logic to tidy up the duplicate code in our TodosController, specifically the multiple instances of Todo.find that are in our #show, #edit, #update and #destroy methods - these all use the same code to find the required Todo to manipulate.
If this doesn't make you sad, it probably should.

Let's tidy this up by adding a new method to do this under in the private block in /app/controllers/todos_controller.rb (as only the controller will use this method):

    def set_todo
      @todo = Todo.find(params[:id])
    end

With a method set to do this, we can now use this to find our Todo objects instead!

However just replacing all of the instances of Todo.find with it would almost be just as bad. Let's instead use some more Rails magic to get this method to run before each of the actions that used it - at the top of our TodosController (just below the class definition line):

  before_action :set_todo, only: [:show, :edit, :update, :destroy]   ## find Todo items

This will tell Rails to run this method before executing the actions that we've specified in the only argument, meaning that we can now remove each Todo.find line from all of their code blocks!
For example, your #show action should now just read:

  def show
  end

 

---

 

The last bit of this section isn't a refactoring step per-se, but is, in my opinion, a vital part of any good web environment: the page title. This is often overlooked as it isn't a part of the web interface itself and is only displayed in your browsers window's / tab's top bar. However, as a shameless power user, a lack of descriptive title proves to be most irksome when I'm in the throes of a process or issue.
Therefore, we will be good developers and ensure that the title of each page at the very least indicates what part of the app we are currently using (as well as the name of the app itself).

One way we could do this would be to extract the action of the viewed page from the page params hash and stick that in the <title> element in our application.html.erb:

    <title><%= params[:action].capitalize %> | ToDoList</title>

When you reload your page, you will see exactly what we have entered in the browser / tab title bar (i.e. Home | ToDoList on the home page).

However this isn't going to be fantastically useful when we're handling individual To-Do items, as each one will show read Show | ToDoList. To get around this, we will utilise the incredibly handy content_for helper offered by Rails by passing it a key and the value we want from each view, and then retrieving this value in the template (application.html.erb) using yield with the same key we used in the helper.

The helper is very clever and takes the result of a block as its value, so we can do some funky things with it if we want. For now, however, we will just pass it the name of the currently accessed To-Do item.
We can add this at the top of the show and edit views:

<% content_for(:html_title) { @todo.name } %>

And then replace the existing <title> element in application.html.erb with logic to display this value if present, as well as the action and the app name as above:

    <title>
      <%=
        [
          (yield(:html_title) if content_for?(:html_title)),
          params[:action].capitalize,
          'ToDoList'
        ].compact.join(' | ')
      %>
    </title>

Note the logic behind this - the value of :html_title will only be added if it is there, else it will return nil which is removed with the compact method on the array. The whole thing is then stitched together using the pipes we passed the join method!
Reloading the browser page again (on a show / edit page) will reveal the fruits of our labours!

 

---

 

With that, we can now go back to our browser and test our fully refactored application, celebrate our tidy code by doing a short dance on the desk chair before pushing changes to GitHub!

 
 

< [ToDoList] The Rails Cyberman	|	[ToDoList] Deploying the App >
Back