Bjoern Geuken napsal(a): > ref: refs/heads/master > commit 995bc764b3e72bdb385961a80e3ee861fd9edec4 > Author: Bjoern Geuken <[email protected]> > Date: Wed Sep 2 10:45:36 2009 +0200 > > status module: code cleanup. TODO: implement limit format with maximum > and minimum > --- > .../status/app/controllers/status_controller.rb | 100 > +++++--------------- > 1 files changed, 24 insertions(+), 76 deletions(-) > > diff --git a/plugins/status/app/controllers/status_controller.rb > b/plugins/status/app/controllers/status_controller.rb > index bd5d22c..d791323 100644 > --- a/plugins/status/app/controllers/status_controller.rb > +++ b/plugins/status/app/controllers/status_controller.rb > @@ -14,63 +14,35 @@ class StatusController < ApplicationController > @permissions = @client.permissions > end > > - def create_data_map( tree, label = "") > + def write_data_group(label, group, metric_name, limits=nil) > + data_list = Array.new > + divisor = (group == "memory")? 1024*1024 : 1 # take MByte for the value > + count = 0 > + data_list = label.map{ |v| count+=1; [count,v.to_f/divisor]} Hi, just few notes, here I don't understand what is label. You have default value for label empty string "", is it really intended? then this map take from String each character and convert it to float (so you get ascii value). From my point of view maybe label is array? > + @data_group[group].merge!({metric_name => data_list}) As I previously say, merge is if you want merge two hash, but in this case you want set one value in hash (it is more effective and less confusing) so you can use @data_group[group][metric_name] = data_list > + divisor = (group == "memory")? 1024*1024 : 1 # take MByte for the value you redefine divisor with same value, is it intended? (second line of this method)
Moving shared function to separete method and use it to reduce code (DRY) is good job ;) Josef -- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
