Best practices

Ruby
1

I’m working my way through the Pickaxe book and I have a question
regarding syntax and best practices.

Example from page 64

 # 1 Book example
 songs.append(Song.new(title, name, mins.to_i * 60 + secs.to_i))


# 2 Alternate version.
 duration = mins.to_i * 60 + secs.to_i
 songs.append(Song.new(title, name, duration))

Version 1 is very concise but harder to read. Ruby is very intuitive
but I find the second example easier to read. What is everyone else
doing?

J-C

2

IMO your second version is better since the intent is clear. IMO,
make the intent clear at the expense of concision.

However, do what works best for you and your team.

3

I like this multi-line nested format, that I call stanza (you know,
like in poetry)

songs.append Song.new(
title,
name,
duration = ( mins.to_i * 60 + secs.to_i )
)

It clearly shows you what you are appending as a chunk of code on the
right. the ‘duration =’ idiom makes the intent explicit. I use
brackets to nest conceptual entities (Song, duration), so that they
stand out as visually striking.

Sometimes, using metaprogramming I also get rid of the explicit new by
wrapping up object instantiation with a Song(…) method on the top
level. I use this Implicit Constructor because I don’t want to be
concerned with object creation in my code, just with the meaning of
the entities that I am manipulating.

songs.append Song(
title,
name,
duration = ( mins.to_i * 60 + secs.to_i )
)

–
Chiaroscuro

Liquid Development: http://liquiddevelopment.blogspot.com/

4

On Thu, 6 Apr 2006, Jean-Charles Carelli wrote:

duration = mins.to_i * 60 + secs.to_i
songs.append(Song.new(title, name, duration))

Version 1 is very concise but harder to read. Ruby is very intuitive but I
find the second example easier to read. What is everyone else doing?

sometimes i warp a bunch of lines into one, but in production code i
tend to
do this:

duration = mins.to_i * 60 + secs.to_i
song = Song.new title, name
songs.append song

otherwise it can be difficult to interpret stack traces with line
numbers.
consider

songs.append(Song.new(title, name, duration))

this can throw in Song.new or append - but both are on the same line.
using a
duration tmp var is also very self doccumenting.

i guess it all just depends on the application and context of the code
being
written.

regards.

-a

5

On 5 Apr 2006, at 21:39, Jean-Charles Carelli wrote:

duration = mins.to_i * 60 + secs.to_i
songs.append(Song.new(title, name, duration))

Version 1 is very concise but harder to read. Ruby is very
intuitive but I find the second example easier to read. What is
everyone else doing?

:slight_smile: I choose the third way. I find the first approach too long, and I
dislike unnecessary intermediate values of the second.

My approach is to factor out the computation of duration in to a
separate method:
def duration; mins.to_s * 60 + secs.to_i; end

Allowing me to write the call as:
songs.append(Song.new(title, name, duration))

I find this helps to make code highly self documenting in many cases.

Cheers,
Benjohn

6

On Apr 5, 2006, at 2:40 PM, Benjohn B. wrote:

I find this helps to make code highly self documenting in many cases.
I prefer that method as well. IIRC, Martin F. also recommends
this (reducing the number of variables) in his Refactoring book. This
may cause performance loss, but optimize later when you know it’s a
problem; and don’t let it keep you from making pretty code.

There are always exceptions of course, and I think it may be a little
over-kill for the simple example, but use your own judgement.

Ryan

7

For me, this dilemma (long one-liner vs. several short lines) is usually
a sign that I need to refactor.

I will always take this:

songs.append(Song.new(title, name, mins.to_i * 60 + secs.to_i))

and write it as

duration = mins.to_i * 60 + secs.to_i
song = Song.new title, name
songs.append song

then refactor it into

def append_song(title, name, mins, secs)
duration = mins.to_i * 60 + secs.to_i
song = Song.new title, name
songs.append song
end

so then the original line way above becomes

append_song title, name, mins, secs

And then I feel great: it’s a one-liner in the upper code, but the code
that does the work is still very readable (and also potentially easier
to reuse).

There’s almost never a code-appearance problem that can’t be solved with
another level of refactoring :slight_smile: (with apologies to Butler Lampson;
http://en.wikipedia.org/wiki/Butler_Lampson)

Jeff

8

Jeff C. wrote:

song = Song.new title, name
songs.append song

then refactor it into

def append_song(title, name, mins, secs)
duration = mins.to_i * 60 + secs.to_i
song = Song.new title, name
songs.append song
end

Where did the ‘songs’ variable come from?

I confess that I’m not so quick to refactor; making three lines into
six, with no real gain in clarity, just isn’t a compelling case for me.

If I find myself repeating code, then sure. But the more common case is
a one-off chunk of terse code that can be made more readable with a
few intermediary variables.

–
James B.

“In physics the truth is rarely perfectly clear, and that is certainly
universally the case in human affairs. Hence, what is not surrounded
by
uncertainty cannot be the truth.”

  • R. Feynman
9

[email protected] wrote:
…

otherwise it can be difficult to interpret stack traces with line numbers.
consider

songs.append(Song.new(title, name, duration))

this can throw in Song.new or append - but both are on the same line.
using a
duration tmp var is also very self doccumenting.

Good points, and very much what I tend to do.

Over time, if I get tired at looking that the same chunk of known-good
code, I may roll stuff up into tersitude, but that’s mainly to make it
easier to focus my eyeballs on something else.

Stupid obvious works real well for me, so unless there is a
significant cost to creating needless intermediary objects, I’d rather
the code be explicit. Makes errors easier to pinpoint, and makes it
more likely Mr. Britt will be able to work with is own code in the
future.

A little more typing today can mean a little less thinking later on.

–
James B.

“In physics the truth is rarely perfectly clear, and that is certainly
universally the case in human affairs. Hence, what is not surrounded
by
uncertainty cannot be the truth.”

  • R. Feynman
10

Jean-Charles Carelli: I always go for a little more verbosity rather
than sacrifice clarity or readability in an attempt to compress code
into the most terse articulation. I use whitespace and
my-typing-speed-is-fast-enough-to-type-long variable names, and break up
long lines with newlines and indentation (as per Chiaro Scuro’s
examples).

Jeff C. wrote:

def append_song(title, name, mins, secs)
duration = mins.to_i * 60 + secs.to_i
song = Song.new title, name
songs.append song
end

so then the original line way above becomes

append_song title, name, mins, secs

This sort of throws up a warning flag for me. :slight_smile: Code like that makes
it feel like PHP’s
becauseweshyawayfrom_objectsandnamespaces_inthisdomain_wewilldothis_withthesethings(
args… ), and moves away from myobject.mymethod.myothermethod( args…
).

I would still keep songs.append song (or songs << song).

Pistos

11

Many thanks for the feedback. The general consensus is: Keep it
readable !

I chose to learn Ruby for many reasons. Two big reasons were: the
Pragmatic guys like Ruby, and Martin F. likes Ruby.

So, as I work through the Pick Axe book, I feel like the code
samples don’t follow in the tradition of what Martin F. or the
Pragmatic guys would recommend.

This is not an attack of the Pick Axe book. It’s great, and I’m
learning things quickly. My main concern was that I was learning a
language that appeared to be written in as cryptic a manner as possible.

Yes, I know I can do as I like. But just as two space indents are
recommended, I want to write code other Ruby developers will feel
comfortable with.

So, I’ll refactor, break things up, and I will not be afraid to be
verbose.

Cheers!

J-C

12

John C.:

I would ALWAYS refactor that to…
title.gsub!(/_/, ’ ')
songs.append(Song.new(title, name, mins.to_i * 60 + secs.to_i))

Which is especially true because gsub! returns nil if no substitutions
were performed. Most of the bang methods are really dangerous - but for
another reason than widely believed.

cheers

Simon

13

I can take and I also do either. I tend to wrap around 70 characters.

That probably is because I learnt to program with punch cards, :slight_smile: but
may
be because it actually is more readable.

One hard and fast rule. NEVER BURY SIDEEFFECT EXPRESSIONS DEEP IN AN
EXPRESSION.

For example…

If you had…
songs.append(Song.new(title.gsub!(/_/, ’ '), name, mins.to_i * 60 +
secs.to_i))

I would ALWAYS refactor that to…
title.gsub!(/_/, ’ ')
songs.append(Song.new(title, name, mins.to_i * 60 + secs.to_i))

On Thu, 6 Apr 2006, Jean-Charles Carelli wrote:

duration = mins.to_i * 60 + secs.to_i
songs.append(Song.new(title, name, duration))

Version 1 is very concise but harder to read. Ruby is very intuitive but I
find the second example easier to read. What is everyone else doing?

J-C

John C. Phone : (64)(3) 358 6639
Tait Electronics Fax : (64)(3) 359 4632
PO Box 1645 Christchurch Email : [email protected]
New Zealand

Carter’s Clarification of Murphy’s Law.

“Things only ever go right so that they may go more spectacularly wrong
later.”

From this principle, all of life and physics may be deduced.

14

On 4/5/06, Ryan B. [email protected] wrote:

I find this helps to make code highly self documenting in many cases.

I prefer that method as well. IIRC, Martin F. also recommends
this (reducing the number of variables) in his Refactoring book. This
may cause performance loss, but optimize later when you know it’s a
problem; and don’t let it keep you from making pretty code.

There are always exceptions of course, and I think it may be a little
over-kill for the simple example, but use your own judgement.

Thanks for pointing this out. Until I read “Refactoring,” I used to be
a big fan of the temp variables approach (in Perl, I’m new to Ruby)
because it is really much easier to read, but it also has a sort of
temptation for the maintenance programmer lurking there. There’s this
handy little variable floating around which could be used to store
pretty much anything. If you use a method rather than a temp var, you
still get all the self-documenting stuff, but in a format that’s a lot
less susceptible to misuse.

15

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jean-Charles Carelli wrote:

duration = mins.to_i * 60 + secs.to_i
songs.append(Song.new(title, name, duration))

Version 1 is very concise but harder to read. Ruby is very intuitive
but I find the second example easier to read. What is everyone else
doing?

I agree with Jeff C. on the refactoring if (as James B. points
out) you find yourself repeating this code in anyway. I also
agree with Benjohn B. on the use of a duration method, although i
think it belong on Song, and not just as some generic.

def duration; @mins.to_s * 60 + @secs.to_i; end

songs.append(Song.new(title, name, duration))

The above code doesn’t work for me. Duration is a responsibility of the
Song IMO. Move duration as an instance method on Song, and
then forget about it elsewhere. This is of course if you have to
constantly calculate the duration for the Song constructor. If
you always have minutes and seconds and have to compute the duration,
why not just let Song take care of it for you? Do you ever
just know the “duration” without having to calculate it ?

class Song
def duration; @mins.to_s * 60 + @secs.to_i; end

def initialize( title, name, minutes, seconds )
   @title, @name, @min, @secs = title, name, minutes, seconds
end

end

Zach

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFENk6BMyx0fW1d8G0RAqb3AJ4xQNzn2LCjfuCausT1fozCXmESWQCfdHpx
hSke8mbx7gRyM6L93AAZp9s=
=Xg1B
-----END PGP SIGNATURE-----