- Controller actions doing too many things.
- Instead, move additional processing code into private controller methods. If private processing methods are growing (making the controller fat) consider moving that code into Ruby classes that are in a sub directory with the same name (namespaced) as the controller.
- Controller actions performing business logic by manipulating internal state of models.
- Instead, move that business logic into methods in the model that the controller invokes.
- Controllers contain too many non-RESTful actions.
- This is a smell that there may be another hidden resource that needs to be exposed into it’s own controller.
- Controller actions are looking up the same values in the params hash.
- Instead, move lookup into private method that all actions can reference.
- Model callbacks (i.e. before_safe) are calling/creating other objects.
- Callbacks should only manipulate internal state of the model. This smell causes unnecessary coupling between models and unexpected side effects when interacting with models. Instead, move any code that needs to interact with other objects into middleware objects that handles object orchestration.
- Model doing too many things.
- This breaks the single responsibility principle . Instead, move the logic for a specific process to non-ActiveRecord Ruby classes. This approach also makes it easier to write unit tests.
- Model class methods (i.e. def self.recent_comments) that are wrappers for more complex ActiveRecord queries.
- Instead, move this code into ActiveRecord scopes. Scopes are internally in ActiveRecord just class methods, but scopes have the advantage of being chainable. Meaning, that scopes always return a ActiveRecord Relation.
- Views have too much display logic or worse business logic.
- Instead, move display logic into Rails helpers and any business logic into the model, refer to #2 under Controllers above.