From: Glenn Smith <glenn.ruby@...>
Date: 2005-04-09T04:12:29+09:00
Subject: How should this really be coded in the Ruby style?

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


	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


-- 

All the best
Glenn
Aylesbury, UK