Clemens Kofler's Profile

I wonder why you always seem to use Time.zone.now, Ryan. Time.current achieves the same thing in Rails: It uses Time.zone.now if Time.zone is set and otherwise falls back to Time.now. See http://apidock.com/rails/Time/current/class.

A note on the destroy action: You should really use Rails' dom_id helper method here because AFAIK that's what all of Rails' other helper methods (e.g. content_tag_for, form_for etc.) use. So the code would be like this:

$('<%= dom_id(@task, :edit) %>').remove();

Yes, but as usual, it's not 100% cross browser compatible. It is supported by Chrome, current versions of Firefox and the newest version of Safari (see https://github.com/blueimp/jQuery-File-Upload/wiki/Browser-support at the bottom of the page).

article.tags.map(&:name).map { |t| link_to t, tag_path(t) }.join(', ')

should really be

article.tags.map { |t| link_to t.name, tag_path(t.name) }.join(', ')

The original code will produce unnecessary iterations that might start to affect performance once the number of tags increases. In general: Keep iterations to a minimum – in my experience, it's the number one performance problem of most Rails apps.

Just a note (although it's already mentioned in the episode): The code of the voted_for? method is inefficient. The problem is that it actually instantiates an ActiveRecord object which, in this case, is rather unnecessary.

Instead of this

def voted_for?(haiku)
  evaluations.where(target_type: haiku.class, target_id: haiku.id).present?
end

you should use the exists? method:

def voted_for?(haiku)
  evaluations.exists?(target_type: haiku.class, target_id: haiku.id)
end

If you are using scopes or something like that, you can also suffix it to a relation:

def voted_for?(haiku)
  evaluations.where(target_type: haiku.class, target_id: haiku.id).exists?
end

In both cases, rather than instantiating an ActiveRecord object, it would just call the connection's select_value which is way more efficient.

Side note: The exists? and its sibling any? are also helpful for use cases where you want to render a collection or, if it's empty, show the user a message like "No comments yet" or whatever.

A little note: Instead of using the set_default method here (or Capistrano's own _cset), a similar effect can be achieved by using the second parameter to the fetch method like so:

directories_to_create = fetch(:directories_to_create, [])

If I now use set(:directories_to_create, Dir[...]) that gets used, otherwise it uses the second parameter as the default (in this case just an empty array).