From: RichardOnRails <RichardDummyMailbox58407@...>
Date: 2007-11-21T11:35:02+09:00
Subject: Re: False positives in editing data

On Nov 19, 2:23 am, Ryan Davis <ryand-r...@zenspider.com> wrote:
> I realize you solved your problem, but there is a much more insidious
> underlying issues here:
>
> On Nov 18, 2007, at 22:40 , RichardOnRails wrote:
>
> >> MLN = 2 # MaxLeadingNumbers
>
> MLN
>
> >>        sName =~ /^([\d]+)?\.?([\d]+)?\.?([\d]+)?\.?/
>
> sName
>
> >>        n = Array.new
>
> n
>
> >>        print "DBG0>> ";  (1..MLN+1).each       { |i|
> >> printf("\t#{n[i]}") };
>
> nasty compounded lines and obtuse debugging output
>
> >>                i_thNumberExists = n[i] ? true : false
> >>                iThNumbrerHasLeadingZero = i_thNumberExists &&
>
> i_thNumberExists
> iThNumbrerHasLeadingZero
>
> ARGH! My eyes! STOP!!!
>
> Ruby is NOT C.
>
> For the love of all that is ruby, please write readable code!
>
> Use English variable names (or whatever your native language is--just
> use words). Don't use hungarian notation, it doesn't help--hell, your
> variable names are so ugly you can't even read the misspellings in
> them. Don't mix camelcase and underscores. Don't use 'n[i] ? true :
> false' when 'n[i]' would do. Don't debug with logging. Write tests. /
> [\d]+/ is the same as /\d+/. Those evals are TERRIFYING. Don't use them.
>
> And, most importantly, 2 spaces per indent.
>
> If you follow these suggestions, your code will be much more
> understandable and less error prone. Really.

Hi Ryan,

Thanks for taking the trouble to critique my code.  Since you were
kind enough to take a thorough look at the code and offer detailed
comments,  I feel obligated to address each issue you raised.

I agree with some of your responses, don't understand some, and
suspect some are merely a matter of taste.  I'm going to tackle them
one-by-one,  but I especially want to thank you for suggesting Unit
Test in Ruby, which I haven't used yet (and don't immediately see how
I'd apply them).

> >> MLN = 2 # MaxLeadingNumbers
>
> MLN

Are you suggesting that rather than using an abbreviation for
MaxLeadingNumbers, I should use the longer string?  If so,  don't you
think that would create unnecessary clutter?  Is this more than a
matter of personal taste?

>
> >>        sName =~ /^([\d]+)?\.?([\d]+)?\.?([\d]+)?\.?/
>
> sName

Did you draw attention to this because of the Hungarian notation?  If
so,  do you think I'm unwise to adopt the style once advocated by
Charles Simoni, super-programmer and co-founder of a giant software
company?

> >>        n = Array.new
>
> n
>
> >>        print "DBG0>> ";  (1..MLN+1).each       { |i|
> >> printf("\t#{n[i]}") };
>
> nasty compounded lines and obtuse debugging output

There was one more piece of that line that got cut off somehow:
"puts".  If we get aside from the tracing issue for a moment, don't
you think it's better to have one-liner debugging statements that can
be easily deleted later,  rather than multi-line statements where
something might be overlooked when deleting them later?

> >>                i_thNumberExists = n[i] ? true : false
> >>                iThNumbrerHasLeadingZero = i_thNumberExists &&
>
> i_thNumberExists
> iThNumbrerHasLeadingZero

To me,  "i_thNumberExists" reads naturally in English as "i'th Number
Exists".  I stated out with iTh ... and thought it less readable.  I
eventually replaced all instances of iTh,  but posted my code before I
completed that correction.  My apologies;  I agree that had I not made
them uniform, they would provide risks of misspelling.

> Ruby is NOT C.

I'm a retired software developer with experience with a lot of
languages,  but by far the most experience with C and C++ under
Windows.  So it takes a little while to adopt the idioms of new
language.

> Use English variable names (or whatever your native language is--just
> use words). Don't use hungarian notation, it doesn't help
> --hell, your
> variable names are so ugly you can't even read the misspellings in
> them.  Don't mix camelcase and underscores.

I addressed those issues above.

> Don't use 'n[i] ? true :  false'
> when 'n[i]' would do.

Agreed. In this case,  I wasn't sure that Ruby was honoring that
equivalence,  so I wanted to make it explicit.

> Don't debug with logging. Write tests.

GOOD POINT!

> /[\d]+/ is the same as /\d+/.

Thanks.

> Those evals are TERRIFYING.  Don't use them.

How else can you loop through $1, $2 ... without repetitive code?

> And, most importantly, 2 spaces per indent.

Agreed.  I thought I did that when I posted.

> If you follow these suggestions, your code will be much more
> understandable and less error prone. Really.

I'll keep my fingers crossed :-)

Regards,
Richard