Special Note:

I asked a very similar question on Stack Overflow, but I have since decided the problem should be solved from a design standpoint rather than an implementation standpoint. I have removed a lot of the coding details from my original problem, as well as concerns about where individual bits of logic should go. I do think I will close my question on Stack Overflow. I am also welcome to advice or criticism if you would like to comment on this decision, as that will only help me in the future. Thanks!




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

I am using Rails to develop my current project and as it stands now, all of my application logic in is the controllers. The models only contain information defining the relationships between tables. 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

Part of some website allows users to search for some things. Let's call these things - neatObjects. The user doesn't know exactly how the results are matched against their input.

This is similar to the way many searches work. Take YouTube for example: you enter a search input and videos are shown to you that match your input. These videos probably have your input in their title, but you may also see videos whose titles don't exactly match your input. Your input is probably being tested against video descriptions, tags, authors, and the results may be influenced by your recently watched videos, or searches, as well as many other factors. The point is, the user doesn't know exactly how these videos are found, but is happy because it works. The same scenario is happening here.

Some development/design team has decided how to match neatObjects to user's inputs, and now some poor full-stack developer who has been given the job of implementation, has to decide how to organize the logic between the controller and the model.

It isn't important to focus on the details of my particular case, but to be able to talk about concrete examples, I will describe some details:

View
The view consists of two pages, a user inputs a string on the search page, and then views results that match on the view page.

Controller
A controller serving the view will need to make the list of results available to the view page.

Model
The model at least defines the relationship between neatObjects and other tables in the database.

Database
The database stores an id, a description, an address, and a date_created in the neat_objects table.


The matches test

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/UX 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.


Sample Implementation

The search functionality can easily be implemented in either the NeatObject model, or the controller serving the view. I don't want to get caught up on the code, but I wanted to include it so that each step in this process is outlined. I have included both code samples so that individual parts of either implementation may be referenced by myself or anyone responding to the question, if need be.

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

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  



#searches_controller.rb

def search
    @neatObjects = NeatObject.get_matches_for( params[:searchString] )
end



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 in the article 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. And what if that test changes?

I feel controller should just be able to say People.find_recent or Message.find_recent and know it got the correct results. So, it should also be able to say People.find_someOtherTime or People.find_someOtherOtherTime, right? But once you need to handle several cases, you should just put them together in one method, right?

The author of that article also pointed out something that brought about a lot of confusion and concern for me. He said that you could alter the find_recent method to take in a parameter to specify recent like so: find( :recent ). (Ignore that fact that find is already defined in Rails.) That makes sense, but how many different arguments do you account for? Just recent, and not_recent? Or recent, last_month, last_year, and really_old? At some point this just becomes a search function, right? And then we are back to problem this post is about.

My biggest concerns/questions:

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, since the data is not dependent on 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.

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

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

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?

  • If the model is concerned only with the data, how can one justify passing user input to it? Especially if the model has to clean up that input before it can even use it.

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?

What about more complicated:

  • 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 for MVC?

  • What guidelines can be followed when you find a gray area, or are unsure about where logic belongs?

And most importantly and least answerable:

  • Does search logic belong in the controller or the model?

The View is responsible for displaying data and accepting input from the user.

The Controller is responsible for accepting requests and directing them to the proper method in the model for performing the requested operation, including related operations such as granting or denying security access and form validation. It is also responsible for directing the resulting output to the View.

The Model is responsible for everything else.

  • I really like how your answer provides a solution for any situation since you end it by directing all other tasks to the Model. – Matthew Cliatt Apr 11 '16 at 5:37
  • 1
    In UI design when doing client/server application, the controller is also often responsible for doing a validation check in Forms. Which is of course duplicated in Model. – Walfrat Apr 11 '16 at 8:32
  • This answer makes it sound like the model should be some kind of God class. Passing the correct search algorithm to the model could certainly fall into the realm of responsibility for the controller. – RubberDuck Apr 11 '16 at 15:56
  • @RubberDuck: What do you mean by "passing the correct search algorithm to the model?" I never said anything about a God class; the model is never a single class. – Robert Harvey Apr 11 '16 at 18:38
  • You and I know that @RobertHarvey, but that's not the impression that your answer gives. – RubberDuck Apr 11 '16 at 19:01

Does search logic belong in the controller or the model?

Neither and both.

In the circumstances that you describe, you actually have a need for several different search algorithms, along with the need to determine which one to use at any given time.

This tells me that the model's Search() method needs to take in the algorithm as a dependency.

Search( ISearchCriteria criteria, ISearchAlgorithm algo )

Note: You could also have an overload that only needs an ICriteria. In that case the model would instantiate some default algorithm.

Then the controller would pass in the proper implementation of a search algorithm based on the context you're calling it in.

SearchRecent( string query )
{
    //...

   ISearchAlgoritm algo = new RecentSearchAlgorithm()

    foo = model.Search(criteria, algo)
}

Search( string query )
{
     //..

    foo = model.Search(criteria)
}
  • What is the point of passing a search algorithm to the model, if the model just executes that algorithm? Why not execute it and not pass it to the model? – Matthew Cliatt Apr 11 '16 at 17:06
  • 1
    This is all good and well, but where do you think the controller is getting the search algorithm from? The model, of course. – Robert Harvey Apr 11 '16 at 18:40

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.