Bill Guindon
4/8/2005 7:42:00 PM
On Apr 8, 2005 3:12 PM, Glenn Smith <glenn.ruby@gmail.com> wrote:
> I'm now in the lucky position of finding myself coding in Ruby all
> day! I've spent the last 4 or 5 days developing a 'rails' app which
> I'm hoping to demo to the customer on Thursday!
>
> However, I'm still relatively new at Ruby, though have been a
> programmer for 20+ years.
>
> The thing that is most worrying me at the moment is that when I write
> Ruby code, at the back of my mind I'm wondering if I'm writing it in
> the style of, say, a 'VB' programmer (which I have been for about 10
> years), when there is a better 'Ruby' style which I've not yet picked
> up.
>
> Today I was writing a long bit of code and the longer it got the more
> worried that I was writing it incorrectly. That is, it would be
> perfectly good style in the more traditional-style languages I use -
> C, VB, PL/SQL etc. But everything I've read about Ruby suggests short
> functions, 'DRY', and an OO style.
>
> Below is an example of code I've written today. I'm not asking
> anybody to debug it (I know it has a problem or two in there
> somewhere) , solve problems, etc. What I'm wondering is, can somebody
> glance at this code and say "yes you are writing ruby in the correct
> form' or 'no, you should really restructure this way or that way'.
> It's just one (private) method cut from a Rails controller class.
>
> In particular, similar code sort of repeats itself, which makes me
> wonder if it could be better structured (using a 'proc' maybe?).
>
> Anyway, over to the guru's.
>
> G
I'm hardly a guru, but I'll toss in my $.02
I'm not really adding much, mostly use of 'unless' and 'empty?'
But by using them, you can make your code more like simple english.
> def save_activitydates(change_sent, change_received, org, year)
> return true if !(change_sent or change_received)
return true unless change_sent or change_received
> begin
> activitydates = @params['activitydates']
>
> if change_sent
> if !defined?(activitydates['sent']) or
> activitydates['sent'].strip.length == 0
unless defined?(activitydates['sent']) ||
activitydates['sent'].strip.empty?
had to change the 'or' to '||' to avoid the method missing error if it's nil.
> date_sent = Time.now
> else
> res = ParseDate.parsedate(activitydates['sent'])
> date_sent = Time.local(*res)
> end
> if !defined?(activitydates['due']) or activitydates['due'].strip.length == 0
> date_due = Time.now + (21 * 24 * 60 * 60) # Add 21 days by default
Might pay to set a constant for DAY_IN_SECONDS instead of calculating
it each run.
> else
> res = ParseDate.parsedate(activitydates['due'])
> date_due = Time.local(*res)
> end
> end
>
> if change_received
> if !defined?(activitydates['received']) or
> activitydates['received'].strip.length == 0
> date_received = Time.now
> else
> res = ParseDate.parsedate(activitydates['received'])
> date_received = Time.local(*res)
> end
> end
>
> new_activitydates = Activitydate.new do |ad|
> ad['org_id'] = org.id
> ad['year'] = year
> ad['sent'] = date_sent
> ad['due'] = date_due
> ad['received'] = date_received
> end
>
> if !new_activitydates.save
> raise
> end
raise unless new_activitydates.save
> true
> rescue
> false
> end
> end
>
> --
>
> All the best
> Glenn
> Aylesbury, UK
>
Hope some of that helps. Welcome to Ruby, and good luck with your demo.
--
Bill Guindon (aka aGorilla)