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.

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”?

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.

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

Martin B. 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,
–Â
Marnen Laibow-Koser
http://www.marnen.org
[email protected]

Aleksey G. 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

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