Where to put large application controller code

Hello,

A common experience: You start to develop a new rails application. When controller method code is growing, you will put parts of its code under the private section at the end of the controller, because you want the public methods to reflect the processing logic, not the calculation detail. Later on, this private controller section is also growing. You detect that it consists of many private methods, which could be grouped thematically. What is best practice now? Creating my own classes and putting them into the rails lib directory? These classes would be of singleton type, because it would make no sense to instantiate them more than once: they are just a collection of thematically grouped methods, not really an "object".

Instead of putting the classes under the lib directory, one could also create app/my_logic/ directories, and put the classes there. Is there any difference? Of course, in the last case, one would have to tell rails to also look into the my_logic directories...

What's your experience? What is best practice, to keep the overview over your code?

Thanks and regards, Martin

If your controllers are getting fat it might be an indication that you are putting a lot of the logic in the wrong place and not using the MVC architecture the way its ment to be used. "Skinny controller, fat model" is the practice most try to follow. The "calculation detail" usually belongs in a model.

The article that coined the phrase: http://weblog.jamisbuck.org/2006/10/18/skinny-controller-fat-model

Sharagoz -- wrote:

If your controllers are getting fat it might be an indication that you are putting a lot of the logic in the wrong place and not using the MVC architecture the way its ment to be used. "Skinny controller, fat model" is the practice most try to follow. The "calculation detail" usually belongs in a model.

Are you suggesting to put "everything" into a model, even if it's not related to a database object? So to also put classes into the the app/models directory, which are not of type ActiveRecord::Base ?

Just as an example: I'm using the FasterCSV gem, in order to read uploaded CSV files into a hash table structure, and do a lot of post-processing with this data. I thought it would be more logical to create a separate class in the rails lib directory, and then use its methods from within my controller.

Code that doesn't fit neatly into a model, view or controller, or that is shared across multiple models, views or controllers, should be extracted into a module most of the time, and included where it needs to be. So if you have some controller code like the CSV processing that you mentioned, it could be a good idea to create a module called something like "CsvProcessor", extract the functions into it, and include it in the controllers/models that uses it.

Even if its only used by one model/controller it might still be a good idea to extract it, to avoid messing up the core functionality of that model/controller.

Are you suggesting to put "everything" into a model

Not if it isnt related to a model, that makes no sense. I dont always keep model related code in the model either. I start out by putting "everything" into the model, and if/when a model starts to get messy, I begin extracting things to keep the core concerns to themselfs.

Sharagoz -- wrote:

Code that doesn't fit neatly into a model, view or controller, or that is shared across multiple models, views or controllers, should be extracted into a module most of the time, and included where it needs to be.

Thanks, that's really what I needed. To put all the code into a module lib/csv_processor.rb , and in the application controller "include CsvProcessor". As this code before was in the "private" section of the controller, is there no danger that the module's methods are now "visible" and could be mistaken as "actions"?

Martin Berli wrote: [...]

Are you suggesting to put "everything" into a model, even if it's not related to a database object? So to also put classes into the the app/models directory, which are not of type ActiveRecord::Base ?

Often, yes. Models are not only for database objects. Read up on the role of the model in MVC: basically it represents *any* domain object. In Rails, that usually means AR subclasses, bur it doesn't have to.

Just as an example: I'm using the FasterCSV gem, in order to read uploaded CSV files into a hash table structure, and do a lot of post-processing with this data. I thought it would be more logical to create a separate class in the rails lib directory, and then use its methods from within my controller.

Your controller should probably not be doing CSV postprocessing. That's more of a model-type responsibility.

Best,

Hi Martin,

That's a pretty common situation that you describe. One way, which is adopted by Rails team is to factor your related methods in modules and include them in your controllers. You can put those modules near your controllers or under /lib (which is loaded automatically).

But before doing that I would also consider moving your logic to models to keep your controllers skinny. The fact that you have many private methods signals that you probably have a lot of logic in your controllers. That's not a very good thing unless there's no other choice.

- Aleksey

I'm sorry, looked at the wrong page and didn't notice the ongoing discussion.

As you could see, I second that you don't need fat controllers and that CSV processing logic belongs to a CSVProcessor model (not module). It doesn't have a table, but it is a standalone entity that knows how to work with data. Isolate your code in it, test it with unit tests and call from your skinny controller.

- Aleksey

Aleksey Gureiev wrote:

I'm sorry, looked at the wrong page and didn't notice the ongoing discussion.

As you could see, I second that you don't need fat controllers and that CSV processing logic belongs to a CSVProcessor model (not module). It doesn't have a table, but it is a standalone entity that knows how to work with data. Isolate your code in it, test it with unit tests and call from your skinny controller.

- Aleksey

So you suggest to create a class app/models/csv_processor.rb which does the job? Which also contains "require 'fastercsv'"? Would you make a singleton of it, as it makes no sense to have several instances? Otherwise in my controller, I would have to write

  cp = CsvProcessor.new   csv_table = cp.create_csv_table(params[:file_ref])

I'd prefer not to have to create an instance here, and just use

  csv_table = CsvProcessor.create_csv_table(params[:file_ref])

Does this make sense?

- Martin