From what I've read, people say the controller needs to be skinny. I was looking into one of the popular rails app discource and here is a controller from that app.
As we can see there are action which are 30 lines long. Is that ok to have? Any ideas feedback and suggestions on this?
Personally, I think yes, because the code shown is clean and understandable. Speaking of skinny, one thing I want to mention here is that you don't really want to make it as short as possible, but to make it **HUMAN READABLE**. The discipline of writing controller is to handle request and response and leave logic to model layer or viewmodel layer.
Since you've mentioned 30 lines I guess your app is doing something more complex than trivial CRUD so... I find TDD to be a great guide. If you're writing a controller spec (mocking approach, as explained in the original RSpec book), than anything longer than ~5 lines is usually too much, because writing a sensible spec becomes just too hard. Have controller only pass params to other objects which live in lib or wherever your domain logic is.
Just because a popular open source app does it, does not imply this is a good practice. In general a good deal of business logic in the controller is a bad.
In this particular example, there is simply no good reason for having the logic of deducing watched count from the database in a controller - it should be a model method ( TopicUser.watched_count_in_category(category_id) ).
It is ideal if all the model operations are delegated to a service object, and controller's responsibility reduces to extracting params from the request and redirecting to appropriate target or rendering the appropriate template. This is simply because Rails does not provide very good mechanism to reuse controllers (shared concerns with business logic gets unwieldy pretty quickly), and controllers are by implementation coupled with HTTP.
It is very common, in practice, to move (or reuse) logic from HTTP request-reponse flow to background tasks or cron jobs. Such operations should be trivial and not require complex refactoring of codebase. This can obviously not happen if the business logic is coupled with HTTP context.
I'm still fresh in ROR, but you can't evaluate a controller from it's length. As controller do what it should do in any number of lines it's fine.
But never forget DRY Concept, also it's better to test many ways to reform your controller.
Processes measured on Time cost and Performance. less time = better code
I don't think lines matter as much as purposes... I try to follow SOLID principles wherever I possibly can. I.E. have modules (controllers in this case) performing as few separate purposes as possible. I'd rather have more controllers or separate logic out of there than controllers with actions that do 10 different things because when one of those things needs to be modified, now everything breaks.
Marcos Bérgamo
Backend Developer
Your controller are responsible to just delegating the responsibility to someone. When you've some complex business rules, are recommended you have another abstraction for you business rules and this complexity the in that layer.
But, in general, depending on your app specific, there ok to prototype the rules inside controller to not introduce an unnecessary complexity on the stage 0 of the app, but when it grows are good to create the business layer and do the right responsibility in the right layer.