Rails MVC - Should database search logic go in the model or the controller

Question

I would like to make sure my code is properly organized/designed according to the following paradigms/patterns:

- Model View Controller

- Rails Convention over Configuration

- Skinny Controller, Fat Model

I have listed them here in the order I think is most important.

---

My issue

After reading several articles, in particular this one and this one too, I started to move some of the logic in my controllers, to my models.

However, I can't decide whether or not to move my searching logic (explained in depth, later) from the controller to the model, even after reading more posts/articles like these: MVC Thinking, and Model vs. Controller, Separating concerns and many more not listed here.

---

The scenario

View

Two pages:

• 1 page contains a text field and submit button which sends the user's input as an argument in params in a POST request to the second page.

• The second page simply renders each neatObject in a given array, let's call it @coolList.

Controller

• A method, let's call it awesomeSearch, is invoked when the second page receives a POST request. (Thanks to Rails Routing)
• awesomeSearch should take the user's input, available as params[:searchString], and work with the NeatObject model to build the @coolList and make that list available for the view to render.

Model

• The NeatObject model handles requests from controllers and returns neatObjects back to those controllers.

• The NeatObject model defines the relationship between neatObjects and other tables in our database.

Database

These are the attributes that make up each neatObject according to our database:

• id - int
• description - string
• address - string
• date_created - timestamp

---

What's Missing?

How the controller works with the model to get matches for the user's input.

This is the part I am confused about. The logic itself is very simple, but I am not sure which pieces belong in the model, and which pieces belong in the controller.

• Should the controller pass the search string to the model, and the model pass back the results?

• Should the controller ask the model for all of the neatObjects, then only keep the ones that match?

• Is the solution a little of both?

To be able to ask questions about specific bits of the logic, I'll next outline the search process with more detail.

---

The search logic in depth

The process involves finding neatObjects that match the search string. It would be impossible to move on without defining what we consider matches for neatObjects. To keep things very simple, we will say that:

A neatObject matches a search string if the search string is contained within its description or its address, ignoring case and leading/trailing whitespace.

This definition is not guaranteed to be permanent. There are several things that may change our definition. We may need to test against more attributes than just address and description, perhaps if the database guys add a new important attribute, or the UI guys decide users should be able to search by ID. And of course, the opposite of these scenarios would mean we need to remove an attribute from the list of attributes we are testing. There are many situations that could change our definition of a match. We could even have to add or remove logic, perhaps if it is decided that we should only test the first word in the description attribute, or perhaps if we should no longer ignore case.

Now we know what defines a match and we know that our definition may change. Now we can more concretely define the search process.

Here is an outline of the steps:

1. Get a reference to all neatObjects
2. Loop through each neatObject, putting each individual one through the match test
   1. Test passes - add/keep neatObject in results list
   2. Test fails - do not keep neatObject for results
3. Make results available to the view

I will reference these steps as I show possible implementations.

---

Implementation

The search functionality can easily be implemented in either the NeatObject model, or the controller serving the view.

Normally, I would just write all of the logic in the controller, but after I learned about the "Skinny controller, Fat model" design, I thought it definitely applied in this situation. This article in particular made me think to reorganize my code, after I saw the author had implemented a "search-like" function in the model. The author's function did not handle user input though, so I was wondering how it should be handled.

This is how I would have written the code before learning about "SCFM":

Search logic in controller:

#searches_controller.rb  

#This is the method invoked when second page receives POST request
def search
    @neatObjects = NeatObjects.all.to_a  

    @neatObjects.delete_if { 
        |neatObject| !matches?(neatObject, params[:searchString])
    }
end

def matches?(neatObject, searchString)
    if((neatObject.description.downcase.include? searchString.downcase) ||
       (neatObject.address.downcase.include? searchString.downcase))
        return true
    end
    return false
end

This method gets its reference to all of the neatObjects (Step 1) by calling .all() on the NeatObject model. It uses the array function delete_if to perform the match test on each neatObject, and only keeps those that pass (Step 2). Step 3 is accomplished automatically automatically since we store our results in an instance variable in the controller which serves the view.

Placing the logic in the controller is very straight-forward, but when considering the "SCFM" design pattern, it seems very illogical.

I've written another option, in which the controller sends the user's input to a function in the model, which will return the the neatObjects which match the input.

Search logic in Model

#NeatObject.rb  

def self.get_matches_for(searchString)
    all.to_a.delete_if { |neighborhood| !matches?(searchString, neighborhood) }
end

def self.matches?(phrase, neighborhood)
    fields = [neighborhood.name, neighborhood.address]

    fields.map!(&:downcase)
    phrase.downcase!

    fields.each do |field|
        if (
            (phrase.include? field) ||
            (field.include? phrase)
           )
            return true
        end
    end

    return false
end  

This method gets the complete list of neatObjects with all() (Step 1). Just like the first method, the model method uses delete_if to remove array elements (neatObjects) that don't meet a certain criteria (passing the match test) (Step 2). In this method, the controller serving the view would call get_matches_for on the NeatObject model, and store the results in an instance variable (Step 3), like this: @neatObjects = NeatObject.get_matches_for( params[:searchString] )

I do think that the model option is cleaner, and slightly more maintanable, but I'll go more in depth in the following section.

---

Concerns

I can see pros and cons to both the model method and the controller method, but there are things that I'm still unsure about.

When I read over that article I've referenced several times (just like I did here), it was very logical that the model defined a function to return the recently added people.

The controller didn't have to implement the logic to determine if a person had been recently added. It makes sense that the controller shouldn't, because that is dependent on the data itself. There might be a completely different implementation of the "recency" test for messages. The recent people might include people added this week, while recent messages are only those messages sent today.

A controller should just be able to say People.find_recent or Message.find_recent and know it got the correct results.

• Is it correct to say the find_recent method could also be modified to take in a time symbol, and return objects for different time periods? Ex - People.find_in_time( :before_this_month ) or Messages.find_in_time( :last_year ). Does that still follow the MVC pattern and Rails convention?

• Should a controller be able to find matches for user input with NeatObject.get_matches_for( searchString )?'

I think the matching logic belongs in the model, because there are certain/specific attributes that are used in testing, and those attributes are different depending on the data. We might have different attributes for different tables, and those attributes definitely shouldn't be defined by the controller. I know the controller depends on the model, not the other way around, so the model must define those attributes, even if the rest of the logic is in the controller.

• And if the model defines the attributes that are used when searching, why shouldn't it define the entire search function?

The above text explains why I think the model should handle the searching logic and expresses the majority of my questions/concerns, however I do have some opinions favoring the other option.

• If the model is concerned only with the data, how can one justify passing user input to it?

If the controller doesn't handle the search logic, it still needs to send the user's input to the model. Does it clean it up before it sends it? (Removing leading/trailing whitespace and downcasing the string.) Does it just send the exact input it got from the user?

• Where is the testing done to make sure the input is not malicious?

Some of my biggest questions:

• Does the answer for where the logic goes change for each case?

If the search/matching process was simpler, would the code's place change? For example, if the searching was simpler:

• If the only attribute being tested was the address and that was unlikely to change, would we just handle the search with the controller?

• If we were making an advanced search feature, where the user decided which attributes to include in the search and also controlled some other factors, there would be a lot of user input just to define the arguments to the search function. Would that be too much logic or user input to place in the model?

---

In conclusion

• How can I always be sure that I am putting logic in the right place (MVC)?
• For my specific case, with this data and this search/match process, how should I organize the code?
• What guidelines can be followed when you find a gray area, or are unsure about where logic belongs?

Answer 1

This is something that varies a lot between cases (each application is different) but here's what I do on my end (large sports app that searches lots of tables from many different places):

• start by performing small searches with non-repeating code patterns in the controller (or rarely view)
• when it turns out the code is needed in more than one or two action/view I move it to the models

I also move the code to the model when the query complexity goes above the average .find or simple .where

Like this, you don't spam your model with one-time-use methods, and at the same time don't repeat the same code over multiple controllers/actions/views

Answer 2

As many thinks in IT and in life, It depends of the scale and the goal.

Considerations:

1) For design: If you see you are violating DRY in your controllers many times, its probably time to move your logic to models.

2) For performance: Since controllers are loaded more than models, Logic in controller tend to performs worst.

And not less important: Unless you are doing something very trivial, and you db has a few thousands rows, do NOT use db for text search. Instead, use a search engine like Solr, ElasticSearch, Sphinx, etc.