4

I wanted to make a simple chart of users that have been created in the past month in my app. Like basically for each day in the past month I want to show the count of users that have registered that day. What I have so far:

# Controller
@users = User.count(:order => 'DATE(created_at) DESC', :group => ["DATE(created_at)"])

# View
<% @users.each do |user| %>
  <%= user[0] %><br />
  <%= user[1] %>
<% end %>

# Output
2010-01-10 2 
2010-01-08 11
2010-01-07 23
2010-01-02 4

Which is ok, but if no users were created on a given day, it should say "0" instead of not being there at all. How can I loop through each day in the last 30 days and show the count of users created on that day?

JP Silvashy
  • 46,977
  • 48
  • 149
  • 227

3 Answers3

8
date = Date.today-30

# Controller
@users = User.count(:conditions=>["created_at >= ?", date], :order => 'DATE(created_at) DESC', :group => ["DATE(created_at)"])
date.upto(Date.today) do |x|
  @users[x.to_s] ||= 0
end
@users.sort!

# View
<% @users.each do |user| %>
  <%= user[0] %><br />
  <%= user[1] %>
<% end %>
Nakul
  • 1,574
  • 11
  • 13
4

I think separation trumps the minimal performance gains here:

# Controller
@users = User.all(:conditions => ["created_at >= ?", Date.today.at_beginning_of_month])

# View
Date.today.at_beginning_of_month.upto(Date.today).each do |date|
  <%= date %>: <%= @users.select{|u| u.created_at == date }.size %>
end
tfwright
  • 2,844
  • 21
  • 37
  • 1
    Seems like too much logic in the view to me. I think the manipulation belongs in the controller -- and pre-processing seems like a much better idea than `select`ing 30 times. – Emily Jan 11 '10 at 20:32
  • The select is the only additional logic in the view, and it saves the controller 4 lines. My first priority is always skinny controllers. You're definitely right about all the selects, but if that were really a performance issue, I would add a method to User which puts the logic right in SQL. Something like User.new_accounts_per_day. But SQL is hard. But never in the controller. – tfwright Jan 11 '10 at 20:48
  • At most I'd put the "||=0" part in the view, but certainly keep the count in the controller with SQL. That would give you a single line of code (skinny) in the controller, and efficient counting. Your implementation would be O(@users.size), and probably 100x slower for even smallish data sets. – klochner Jan 11 '10 at 21:52
  • You're absolutely right of course. Of course there's no metric for how much clearer my implementation is. I guess I'd rather write the clear, easy solution in 2 mins, come back when a render bottleneck is identified and optimize. – tfwright Jan 11 '10 at 22:57
  • I like this implementation the most. The reason this solution seems a little more skinny than @Nakul's is because Floyd is counting from the `beginning_of_month` instead of the past month (or like past 30 days), therefore his is at most 2 lines shorter. – JP Silvashy Jan 11 '10 at 23:32
  • I guess the one things is that while this is much more readable it is slower for the DB, but I think I can optimize it. My biggest concern is that this would create and N+1 – JP Silvashy Jan 11 '10 at 23:34
  • I think `Date.today.last_month` is better than `Date.today-30` – JP Silvashy Jan 11 '10 at 23:42
3

As @floyd commented on earlier, the code to do the SELECT belongs in the model:

class User < ActiveRecord::Base
  def self.count_new_users_per_day(cutoff_at)
    result = count(:all, :conditions => ["created_at >= ?", cutoff_at],
                         :group => "DATE(created_at)")
    # See http://ruby-doc.org/core-1.8.7/classes/Hash.html#M000163
    result.default = 0
    result
  end
end

The controller does logic and calls into the model layer:

class UsersController < ActionController::Base
  def index
    @cutoff_at = 30.days.ago.at_midnight
    @new_users_by_date = User.count_new_users_per_day(@cutoff_at)
    @dates = ((@cutoff_at.to_date) .. (@cutoff_at.to_date >> 1))
  end
end

And the view is only responsible for displaying the data the controller's setup for it:

# Chose to move the code to a partial
<%= render :partial => "user_count", :collection => @dates, :as => :date %>

# _user_count.html.erb
<td><%=h date.to_s(:db) %></td>
<td><%= number_with_delimiter(@new_users_by_date[date.to_s(:db)]) %></td>

Basically, since SQL won't return missing dates, you have to iterate over the full set of dates yourself, and ask the Hash/ResultSet if it has the correct value. In the model implementation above, I set the Hash's default value after the fact, giving me a clean way to get a zero when a value is missing.

François Beausoleil
  • 16,265
  • 11
  • 67
  • 90