Common Rails Code Smells

When reviewing Ruby on Rails pull requests (PR’s) I find that I frequently come across the following code smells.


  1. 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.
  2. 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.
  3. 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.
  4. Controller actions are looking up the same values in the params hash.
    • Instead, move lookup into private method that all actions can reference.


  1. 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.
  2. 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.
  3. 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.



Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s