Help me refactor: CRUD and fat-model

Question

Hi! I'm trying to wrap my head around these 2 design patterns CRUD and skinny-controller-fat-model by refactoring a real life example. As an example I'm using a (poorly designed) web application that shows countdown to certain events. Here is a screenshot to see what it does. screenshot link

Right now the design is as follows: I have a database table called happenings with 2 attributes (name and date). I have a model in which the countdown to an event is calculated in seconds

class Happening < ActiveRecord::Base
    def secondsFromNow
        self.date.to_time - DateTime.now().to_time
    end
end

I have a controller that enumerates over all the "happening" records of my database and makes a string representing the countdown. If the string equals "completed", it will add the 'happening' to an array called completedHappenings, else it will be added to the array incompletedHappenings. These arrays are passed to the index view template

    def index
        happenings = Happening.all
        #make an array filled with the hashes of happening names,there dates and the time differences
        #this hash will be used to make our view
        @completed, @incomplete = getCounterArrays(happenings)
    end


    def getCounterArrays(happenings)
        completedHappenings = [] 
        incompleteHappenings = []
        happenings.each do |happening|
            #getCountdownString returns a string that either says "completed" or the countdown
            countdownString = getCountdownString(happening)
            if countdownString == "completed"
                completedHappenings << {name: happening.name, date: happening.date, countdown: countdownString, id: happening.id}
            else
                incompleteHappenings << {name: happening.name, date: happening.date, countdown: countdownString, id: happening.id}
            end
        end

        completedHappenings.sort_by! {|hsh| hsh[:date]}
        incompleteHappenings.sort_by! {|hsh| hsh[:date]}
        return completedHappenings, incompleteHappenings
    end

    ##This method calculated the difference in between the Happening date and the current date
    ##It will return a string that either says the difference in between those dates or "completed" 
    def getCountdownString(happening)
        #happening.secondsFromNow is a Model method that returns the countdown in seconds
        differenceInSeconds = happening.secondsFromNow
        if differenceInSeconds > 0
            seconds = (differenceInSeconds % 60).floor
            differenceInMinutes = differenceInSeconds/60
            minutes = (differenceInMinutes % 60).floor
            differenceInHours = differenceInMinutes/60
            hours = (differenceInHours % 24).floor
            days = (differenceInHours/24).floor
            countdownString = makeCountdownString(days, hours, minutes)
        else
            countdownString = "completed"
        end
        return countdownString
    end

    def makeCountdownString(days, hours, minutes)
        if days > 0
            return "#{days} d, #{hours} h, #{minutes} m"
        elsif hours > 0
            return "#{hours} h, #{minutes} m"
        elsif minutes > 0
            return "#{minutes} m"
        end 
    end

And then in the index view template I use those 2 created arrays to display the happenings in the correct list.

<h3>Completed countdowns</h3>
  <ul id="completed-countdowns">
    <% @completed.each do |happening| %>
         <li id ="<%= happening[:id].to_s %>"><label class="eventLabel"><%= happening[:name]%></label><input class="eventInput" type="text">
        <button class="editButton">Edit</button><button class="deleteButton">Delete</button>
        <label class="counterLabel"><%= happening[:countdown] %></label>
        <input type="datetime-local" value=<%= happening[:date].to_s %>></li>   
    <% end %>
  </ul>

One thing is obvious, this can be way better organized and designed. I'd love to fit this in a proper CRUD application, but have no idea what to do with those controller methods that create the countdownString Any help would be so very much appreciated!

Answer 1 · 2015-07-05T18:40:57Z

July 5, 2015 6:40pm

I have an even better suggestion - keep both the model and controller skinny and move some logic to service objects, query objects etc. This article is old, but is a very good starting point: http://blog.codeclimate.com/blog/2012/10/17/7-ways-to-decompose-fat-activerecord-models/

Your methods like getCountdownString are just begging to become service objects. Also, their names should not be in camel case. Rails convention is to use snake case.

Answer 2 · 2015-07-06T08:49:19Z

July 6, 2015 8:49am

Thank you Maciej Czuchnowski , I've read a bit into it and it looks like the better way to go! Still there is some unclarity; In this article they state several criteria for a service model:

Some actions in a system warrant a Service Object to encapsulate their operation. I reach for Service Objects when an action
meets one or more of these criteria:

The action is complex (e.g. closing the books at the end of an accounting period)

The action reaches across multiple models (e.g. an e-commerce purchase using Order, CreditCard and Customer objects)

The action interacts with an external service (e.g. posting to social networks)

The action is not a core concern of the underlying model (e.g. sweeping up outdated data after a certain time period).

There are multiple ways of performing the action (e.g. authenticating with an access token or password). This is the Gang of > Four Strategy pattern.

Could you please elaborate a bit, and explain me under which criteria my problem falls

Welcome to the Treehouse Community

Looking to learn something new?

simon buysse

simon buysse

Help me refactor: CRUD and fat-model

2 Answers

Maciej Czuchnowski

Maciej Czuchnowski

simon buysse

simon buysse

Maciej Czuchnowski

Maciej Czuchnowski