From: Robert Klemme <shortcutter@...>
Date: 2012-01-20T07:22:29+09:00
Subject: Re: Beginner Questions

On Thu, Jan 19, 2012 at 10:46 PM, Ryan Davis <ryand-ruby@zenspider.com> wrote:
>
> On Jan 19, 2012, at 09:13 , T.J. L. wrote:
>
>> Here is a link to what I have:
>>
>> http://codepad.org/mx5Qheyr
>>
>> I'd really appreciate any guidance someone can offer.
>
> Honestly, when you write code that looks like this:
>
>> def saveDB
>>         File.open( $fn, 'w' ) {
>>                 # note the extra line with the massive amounts of trailing whitespace...
>>         |f|
>>         f.write($col_array.to_yaml)
>>         f.write($totaltime.to_yaml)
>> }
>> end
>
> my brain stops reading almost immediately. Your indentation, parenthesis, inconsistencies, and general styling all detract from the readability of the code to the point where it's hard to see the forest through the trees.

I found it difficult to digest as well.

> Here is idiomatic ruby:
>
> def save_db
>  File.open $filename, 'w' do |f|
>    f.write $dives.to_yaml
>    f.write $total_time.to_yaml # tho, this is entirely redundant and could be recalculated
>  end
> end

Another remark: $dives.to_yaml(f) is potentially more efficient since
output is directly written to the stream whereas the form with #write
first creates the complete string in memory before writing it.

Also I find there are too many global variables around.  That limits
flexibility of the code.

Ah, and if total_time is the most important thing to read from the
stream it may make sense to reverse order of fields in the stream,
i.e. write total_time first.

Kind regards

robert


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