Questions on Thoughtbot best practices

I have the following questions regarding Thoughbot Best Practices:

  • Avoid instantiating more than one object in controllers.
    Example?

  • Avoid naming methods after database columns in the same class.
    What about the case you want to do something when the database column is being set?

  • Don’t reference a model class directly from a view.
    Explain please. Classes not instances, right?

  • Don’t use instance variables in partials. Pass local variables to partials from view templates.
    Always? What about the case of simply avoiding the a call to the same path helper multiple times?

  • Don’t use SQL or SQL fragments (where(‘inviter_id IS NOT NULL’)) outside of models.
    What about the case of breaking up a model into smaller classes, maybe creating method objects?

  • Use only one instance variable in each view.
    Always? Is this suggesting anything more than a trivial view always have some presenter object?

  • Use _url suffixes for named routes in mailer views and redirects. Use _path suffixes for named routes everywhere else.
    Is it necessary to fix cases where I used _path for redirects? Why doesn’t (or does) the rails method redirect_to correct a path to a url?

  • Validate the associated belongs_to object (user), not the database column (user_id).
    Example?

  • Use a Fake to stub requests to external services.
    Always? Why not stubs & spies for simple cases?

  • Avoid multicolumn indexes in Postgres. It combines multiple indexes efficiently. Optimize later with a compound index if needed.
    Except when a unique multi-column index is needed!

  • Use CoffeeScript.
    How about for js.erb files? Use coffee.erb files?

Hello @Justin_Gordon. Here are my answers to some of the questions.

class IndexPagesController < ApplicationController
  include ApplicationHelper

  def show
    @index_page = IndexPage.new
  end
end
class IndexPage
  def random_issues
    Issue.randoms
  end

  def categories
    Category.all
  end

  def statistics
    Statistics.new
  end
end

So instead of initializing @random_issues, @categories and @statistic I initialize just @index_page which contains what I need. This is real world example from https://github.com/mrhead/seriemato.sk/.

Same as above. In my view I use just @index_page.

Coffee alternative for js.erb is js.coffee. Such a file will be processed with erb, then with coffee and served as js. I agree that it is misleading and it should be something like action.js.coffee.erb.

class Article
  belongs_to :user

  validates :user, presence: true
  # instead of
  # validates :user_id, presence: true
end

@patrikbona, any reason on the last one for validating the user rather than the user_id?

Where do you put the Page objects? Maybe a subdirectory of where the view is so that all the page objects for a view can be easily found?

Any recommended guides on splitting up code amongst:

  • Haml/Erb: no instance variables, minimal logic
  • Decorator (Draper): Model methods used on many views
  • Page Object: Object to encapsulate data for a page
  • View helpers: Helpers used by many views

I am not from thoughtbot and I did not write the guides, so I cannot answer all your questions. I just gave you examples ;).

Btw. I put all my models to app/models/ so even the Page objects.

Models are not necessarily AR objects. I believe that they refer to models as a more generic term, in which PORO fit.

@Justin_Gordon, you validate the user rather than the user_id because that tells Rails to confirm that there is an actual user associated with the record, and not just a random integer inserted into the user_id column.

1 Like