1b491994d6011d720e332af6e1ffd03b

"pages" is a Hash and contains
{"PRO"=>[:Test, [{"X"=>{"type"=>"dash"}}, {"Y"=>{"type"=>"dash"}}, {"Z"=>{"type"=>"non-dash"}}]]}
(for example)

pages.keys.each do |k|

      logger.debug "Pages for #{k} channel:"

      pages[k].each do |v|

        logger.debug " - *#{v[0].to_s}* with #{(v[1].find_all {|x| x[x.keys.first]['type'] == 'dash' }).length.to_s} dashboard(s)"

      end

    end

Refactorings

No refactoring yet !

880cbab435f00197613c9cc2065b4f5a

danielharan

November 26, 2009, November 26, 2009 16:53, permalink

No rating. Login to rate!

I'd avoid the confusing hash altogether. Here I use OpenStruct, but you could do the same with plain objects.

require 'ostruct'

class Page < OpenStruct
  def dashboard?
    self.page_type == "dash"
  end
end

class Plan < OpenStruct
  def dashboards
    self.pages.select {|p| p.dashboard?}
  end
end

test = Plan.new :name => :Test, :pages => [Page.new(:name => "X", :page_type => "dash"),
                                           Page.new(:name => "Y", :page_type => "dash"),
                                           Page.new(:name => "Z", :page_type => "non-dash")]

test.dashboards.length
74d31ca0235178ff915b2489c9a53ef2

Aaron Scruggs

November 26, 2009, November 26, 2009 17:25, permalink

No rating. Login to rate!

I had trouble getting the above code to execute. Maybe your example hash is slightly off? Assuming not, I starting fiddling with the code and you have tough decision to make. Do you create supporting methods for logging? I went with 'yes'. Also, this seems like an appropriate place to use an "inject" method.

As a side note, if these hashes get large, this could become very expensive to execute when you are not debugging. Wrapping the entire execution in a debug check can save you some cycles.

Happy Thanksgiving! I look forward to seeing what other people come up with.

pages = {"PRO"=>[:Test, [{"X"=>{"type"=>"dash"}}, {"Y"=>{"type"=>"dash"}}, {"Z"=>{"type"=>"non-dash"}}]]} 

def dash_count(array = [])     
  array.inject(0) do |sum, item|  
    sum += 1 if item.values.first["type"] == 'dash'
    sum
  end
end


if logger.debug?
  pages.each do |channel, array|
    logger.debug "Pages for #{channel} channel:"
    logger.debug " - *#{array[0]}* with #{dash_count(array[1]) } dashboard(s)"
  end
end
A8d3f35baafdaea851914b17dae9e1fc

Adam

November 28, 2009, November 28, 2009 04:54, permalink

1 rating. Login to rate!
pages.each do |channel,(name,types)|
  count = types.count { |type| type.values.include?('type' => 'dash') }
  logger.debug "Pages for #{channel} channel:"
  logger.debug " - *#{name}* with #{count} dashboard(s)"
end
D85d44a0eca045f40e5a31449277c26c

Ben Marini

November 30, 2009, November 30, 2009 17:26, permalink

No rating. Login to rate!

I'm with Daniel, the source of the problem is the confusing hash, which is
hard to work with. Here's a more OO approach, similar to Daniel's.

class Channel < Struct.new(:name, :pages)
end

class Page < Struct.new(:name, :items)
  def dash_count
    items.select { |i| i.dash? }.size
  end
end

class Item < Struct.new(:name, :type)
  def dash?
    type == "dash"
  end
end

items    = [ Item.new("X", "dash"), Item.new("Y", "dash"), Item.new("Z", "non-dash") ]
pages    = [ Page.new(:Test, items) ]
channels = [ Channel.new("PRO", pages) ]

if logger.debug?
  channels.each do |channel|
    logger.debug "Pages for #{channel.name} channel:"

    channel.pages.each do |page|
      logger.debug "- *#{page.name}* with #{page.dash_count} dashboard(s)"
    end
  end
end

Your refactoring





Format Copy from initial code

or Cancel