activerecord - Rails Group_By Antipattern -


i encounter code smell when use enumerable group_by method. old code i'm refactoring example

def punctuality_report   params[:date] ? @date = date.strptime(params[:date], "%m/%d/%y") : @date = date.today   params[:through] ? @through = date.strptime(params[:through], "%m/%d/%y") : @through = @date + 1   time_range = @date.to_formatted_s(:db)..@through.to_formatted_s(:db)   @orders = order.daily.includes(:store).where('orders.created_at' => time_range).group_by{|o| o.store}   @orders.each_key.each |s|     eval "@s#{s.id}_early = @orders[s].collect{|o| o if o.early?}.compact"     eval "@s#{s.id}_avg_early = @s#{s.id}_early.count > 0 ? @s#{s.id}_early.collect{|o| o.earliness}.sum / @s#{s.id}_early.count : '0'"              eval "@s#{s.id}_late = @orders[s].collect{|o| o if o.late?}.compact"     eval "@s#{s.id}_avg_late =  @s#{s.id}_late.count > 0 ? @s#{s.id}_late.collect{|o| o.lateness}.sum / @s#{s.id}_late.count : '0'"              eval "@s#{s.id}_on_time = @orders[s] - (@s#{s.id}_early | @s#{s.id}_late)"   end end 

okay i'm coming through , see need refactor report out of action on orders controller , model of own clean implementation logic. there's 1 code smell out, still struggle orders.group_by hash.

the thing when in view layer want hash. need summaries orders need access stores. using group query method in activerecord returns me relation, not useful enumerable group_by hash. feel there's better way need , reduce lot of querying , ruby processing.

i don't see wrong group_by method. see issue overuse of eval though [-; eval strong code smell (imho) group_by

that being said, did see other areas refactor:

# consider refactor:  def punctuality_report   # find more readable   @date = (params[:date]) ? date.parse(params[:date]) : date.today   @through = (params[:through]) ? date.parse(params[:through]) : @date + 1.day    # .in_time_range(range) method can defined scope on order model, , can   # rid of logic here   #    @orders = order.daily.includes(:store).in_time_range(@date, @through).group_by(&:store)  end  class order < activerecord::base    scope :in_time_range, lambda { |date, through| where('orders.created_at' => (date..through)) }    # looks know how collect orders needs... ie order#early, etc  end  # now, consider in views ability this: @orders.each |store, orders|   # orders orders met above qualifications store   orders.early   order.average_of_orders(orders.early)   orders.late   order.average_of_orders(orders.late)   orders.on_time     end 

Comments