"Glenn Smith" <glenn.ruby / gmail.com> schrieb im Newsbeitrag 
news:e09ac11905040812121a23ad3a / mail.gmail.com...
> 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?).

I can't really comment on the structure because I never used Rails.  Some 
minor remarks:

 - Don't use defined? - it's superfluous typing and doesn't gain you a thing 
IMHO.  nil is treated as false in boolean context anyway.

 - Bind (21 * 24 * 60 * 60) to a constant with meaningful name, this is 
faster and saves you the comment

 - On one hand you raise if saving fails but then you catch that exception 
at the end and convert it to a boolean return value.  That's inconsequent 
(and probably imperformant).  I prefer exceptions for such failures but in 
that case the whole begin-rescue-end is superfluous. If you need the boolean 
return, just do any of these

new_activitydates.save or return false
return false unless new_activitydates.save
(I like the first one best because it resembles most to natural language)

> Anyway, over to the guru's.
>
> G
>
>
> def save_activitydates(change_sent, change_received, org, year)
> return true if !(change_sent or change_received)
>
> begin
> activitydates = @params['activitydates']
>
> if change_sent
> if !defined?(activitydates['sent']) or
> activitydates['sent'].strip.length == 0
> 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
> 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
>
> true
> rescue
> false
> end
> end

Btw, I think the indentation got garbled because your original had tabs in 
it - spaces are much safer.

Kind regards

    robert