HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Ruby hashes of status counts

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
countshashesstatusruby

Problem

I have a method that takes a model object, reads through its children and returns a hash of status counts. It should only return a count on a status if one exists.

This method works, but I was wondering if there are a cleaner, more Rubyish way to write it.

The batch_details table is getting very large, so if possible I'd like not to access it twice.

def batch_status
   batch_status = {
      'ready' => 0,
      'processing' => 0,
      'error' =>  0,
      'purchase' => 0,
      'completed' => 0
   }

   self.batch_details.each do |bd|
      batch_status[bd.status] = batch_status[bd.status] + 1
   end   
   batch_status['count'] = self.batch_details.count
   batch_status.delete_if {|k, v| v == 0 }
   return batch_status
end

2.1.2 :002 > b.batch_status
 => {"completed"=>4, "count"=>4}

Solution

You can use Array#group_by to produce a hash keyed by status. Then follow that by a map to collect the counts. You'll get an array of key/value pairs, so use Hash[] to make that into a proper hash:

def batch_status
  counts = self.batch_details.group_by(&:status).map { |k, v| [k, v.count] }
  Hash[counts]
end


Alternatively, you can use use your current approach, just shortened a bit

def batch_status
  self.batch_details.reduce({}) do |counts, details|
    counts[details.status] = (counts[details.status] || 0) + 1
  end
end


Edit: Completely overlooked the "count" key. I muddies the picture a little, but you could do something like this for code blocks 1 and 2 above

def batch_status
  counts = self.batch_details.group_by(&:status).map { |k, v| [k, v.count] }
  counts << ["count", self.batch_details.count]
  Hash[counts]
end


and

def batch_status
  counts = { "count" => self.self.batch_details.count }
  self.batch_details.reduce(counts) do |counts, details|
    counts[details.status] = (counts[details.status] || 0) + 1
    counts
  end
end


Or, as Cary Swoveland suggests:

def batch_status
  counts = { "count" => self.self.batch_details.count }
  counts.default = 0
  self.batch_details.reduce(counts) do |counts, details|
    counts[details.status] += 1
    counts
  end
end


Which cleans it up nicely. Further cleanup could be done by using each_with_object in place of reduce:

def batch_status
  counts = { "count" => self.self.batch_details.count }
  counts.default = 0
  self.batch_details.each_with_object(counts) do |counts, details|
    counts[details.status] += 1
  end
end

Code Snippets

def batch_status
  counts = self.batch_details.group_by(&:status).map { |k, v| [k, v.count] }
  Hash[counts]
end
def batch_status
  self.batch_details.reduce({}) do |counts, details|
    counts[details.status] = (counts[details.status] || 0) + 1
  end
end
def batch_status
  counts = self.batch_details.group_by(&:status).map { |k, v| [k, v.count] }
  counts << ["count", self.batch_details.count]
  Hash[counts]
end
def batch_status
  counts = { "count" => self.self.batch_details.count }
  self.batch_details.reduce(counts) do |counts, details|
    counts[details.status] = (counts[details.status] || 0) + 1
    counts
  end
end
def batch_status
  counts = { "count" => self.self.batch_details.count }
  counts.default = 0
  self.batch_details.reduce(counts) do |counts, details|
    counts[details.status] += 1
    counts
  end
end

Context

StackExchange Code Review Q#76897, answer score: 5

Revisions (0)

No revisions yet.