Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm trying to optimize this bit of code and I thought I'd throw it out here to see if anyone has any ideas. I've been testing the performance using Benchmark.measure {...} and wrapping the entire function.

Yes, the item has a date that is a Ruby Date

  puts Benchmark.measure {
    grouped_items
  }

  def grouped_items
    unless @grouped
      @grouped = {}
      @items.each do |item|
        key = item.date
        if @grouped.has_key?(key)
          @grouped[key] << item
        else
          @grouped[key] = [item]
        end
      end
    end
    @grouped
  end

Thanks for any insight you care to share.

Edit #1: My first optimization. I was aiming for a slightly more succinct function and gained a .2 seconds reduction in time to process 100_000 items.

  def grouped_items
    unless @grouped
      @grouped = Hash.new { |h, k| h[k] = [] }
      @items.map {|item| @grouped[item.date] << item }
    end
    @grouped
  end

Edit #2: My second iteration with more or less the same performance profile but with much less code.

def grouped_items
  @grouped ||= @items.group_by { |item| item.date }
end

Edit #3: An alternative way to do the same thing above.

def grouped_items
  @grouped ||= @items.group_by &:date
end
share|improve this question
Nope. I'll check that out. – Ryan Montgomery Jul 7 '12 at 1:21
In future can you flag for a mod to migrate, please don't cross-post. Thanks. – Kev Jul 8 '12 at 0:58
Did you try profiling the code? – Andrew Grimm Jul 12 '12 at 2:41

2 Answers

up vote 6 down vote accepted

Are you aware of Enumerable#group_by?

def grouped_items
  @items.group_by{|item| item.date}
end
share|improve this answer
No, but I am now. Thanks! – Ryan Montgomery Jul 7 '12 at 1:41
It more or less performs the same as the first edit (see above) but with much less code. – Ryan Montgomery Jul 7 '12 at 1:44
1  
I just came back across this and wanted to comment again how awesome this solution is. :) – Ryan Montgomery Jul 29 '12 at 17:44
Thanks! Your Edit #3 improves on it further. – Mark Thomas Aug 5 '12 at 19:48

What about this?

@items.group_by { |item| item.effective_date }

Benchmarks for the curious:

Ryan v1 : 1.080000   0.000000   1.080000 (  1.077599)
Ryan v2 : 0.620000   0.000000   0.620000 (  0.622756)
group_by: 0.580000   0.000000   0.580000 (  0.576531)
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.