From: Robert Klemme <shortcutter@...>
Date: 2011-05-02T16:27:08+09:00
Subject: Re: update variables based on own value - critique please?

On Sun, May 1, 2011 at 10:44 AM, Peter Ehrlich <crazedcougar@gmail.com> wrote:
> Hey!
>
> First, some background.  The goal here is an experiment to make the best
> code that I can.  My challenge is with returning JSON in an ajax
> request.  In a standard http request, rails view templates would handle
> preparing the data -- pluralizing, capitalizing, whatever.  However,
> there's no view in this case.
>
> The simple approach looks like this:
>
>  def json_for_view
>    rounds['last_pushed']['c_score'] = time_ago_in_words
> DateTime.parse(rounds[:last_pushed][:c_score])
>    rounds['last_pushed']['d_score'] = time_ago_in_words
> DateTime.parse(rounds[:last_pushed][:d_score])
>    rounds['last_pushed']['c_score'] = Date.parse
> rounds[:c_score].to_s(:'January 1, 2011')
>    rounds['last_pushed']['d_score'] = Date.parse
> rounds[:d_score].to_s(:'January 1, 2011')
>    rounds.to_json
> end
>
> I see a couple of problems here:
>  - the field keys are specified first.  This is not DRY.  (etc all the
> headaches of non dry code)

A simple refactoring leads me to

def json_for_view
  %w[c_score d_score].each do |key|
    rounds['last_pushed'].tap do |lp|
      # next line seems obsolete since the value is overwritten
      lp[key] = time_ago_in_words
DateTime.parse(rounds[:last_pushed][key.to_sym])
      lp[key] = Date.parse rounds[key.to_sym].to_s(:'January 1, 2011')
    end
  end

   rounds.to_json
end

>  - A nicer feel would be if there was a method applied to each value,
> like rounds[:last_pushed][:d_score].make_pretty

That will be hard since Hash (or whatever this is) members generally
do not need to know the container.  But that knowledge would be
required to do what you describe above.

> Here's what I've come up with:
>
>
>
> # lib/toolbox.rb
> class Object
>  def send_updates!(hash)
>    # updates values based on blocks passed in a hash
>
>    hash.each do |key, block|
>        self[key] = block.call(self[key]) if self[key]
>    end
>
>  end
> end
>
>
> # the model file
>  def json_for_view
>    rounds = JSON.parse(self.rounds)
>
>    pushed_pretty  = lambda { |score|
> time_ago_in_words(DateTime.parse(score))    }
>    created_pretty = lambda { |score| Date.parse(score).to_s(:'January
> 1, 2011') }
>
>    rounds['last_pushed'].send_updates!({
>      'c_score' => pushed_pretty,
>      'd_score' => pushed_pretty
>    })
>    rounds['created_at'].send_updates!({
>      'c_score' => created_pretty,
>      'd_score' => created_pretty
>    })
>
>    rounds.to_json
>  end
>
>
>
> Thoughts?

Why the lambdas?  You said they should be named but why don't you just
define another method?  Your current solution imposes object creation
overhead for each call and you create new lambdas with identical
content all the time. I'd rather

def pushed_pretty(score)
  ...
end

and use that directly.  There is no reason for a lambda here unless
you want to change the algorithm or even pass it in via a block:

def json_for_view(&pushed_pretty)
  ...
end

> Is this a common problem?

That fact the you need Date.parse and DateTime.parse irritates me.
Normally your data should be in an internal data structure in the
_proper type_.  Parsing from and conversion to string are normally
only necessary at the interface (i.e. when preparing a reply to be
sent or parsing a request).

> Is there a common solution?
>
> Am I doing something wrong which causes this to be an issue?

I don't see the definition of rounds.  This means it's returned via
some method, so this might be better than the code above:

def json_for_view
  rounds.tap do |rd|
    %w[c_score d_score].each do |key|
      rd['last_pushed'].tap do |lp|
        # next line seems obsolete since the value is overwritten
        lp[key] = time_ago_in_words DateTime.parse(rd[:last_pushed][key.to_sym])
        lp[key] = Date.parse rd[key.to_sym].to_s(:'January 1, 2011')
      end
    end
  end.to_json
end

Kind regards

robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/