Forum: Ruby best practices

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Cb6135c82400eadbc936f4cb336e3b9f?d=identicon&s=25 Jean-Charles Carelli (Guest)
on 2006-04-05 22:42
(Received via mailing list)
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
7e6ee7bc26cac7c3f1edce27558ced3d?d=identicon&s=25 Keith Sader (ksader)
on 2006-04-05 23:13
(Received via mailing list)
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.
D8fb06dfc08a477ecb0a76ffdbff3475?d=identicon&s=25 Chiaro Scuro (chiaroscuro)
on 2006-04-05 23:28
(Received via mailing list)
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/
Cb48ca5059faf7409a5ab3745a964696?d=identicon&s=25 unknown (Guest)
on 2006-04-05 23:37
(Received via mailing list)
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
Ffcb418e17cac2873d611c2b8d8d891c?d=identicon&s=25 Benjohn Barnes (Guest)
on 2006-04-05 23:43
(Received via mailing list)
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?

:) 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
085541f9546d0505433183b5f95bbf62?d=identicon&s=25 Ryan Bates (Guest)
on 2006-04-06 01:02
(Received via mailing list)
On Apr 5, 2006, at 2:40 PM, Benjohn Barnes wrote:
> I find this helps to make code highly self documenting in many cases.
I prefer that method as well. IIRC, Martin Fowler 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
Bc6d88907ce09158581fbb9b469a35a3?d=identicon&s=25 James Britt (Guest)
on 2006-04-06 01:23
(Received via mailing list)
ara.t.howard@noaa.gov 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 Britt

"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
8217faf2bfdfa7daf10135d41ddd421e?d=identicon&s=25 Jeff Cohen (jeff)
on 2006-04-06 03:58
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 :-)  (with apologies to Butler Lampson;
http://en.wikipedia.org/wiki/Butler_Lampson)

Jeff
www.softiesonrails.com
Bc6d88907ce09158581fbb9b469a35a3?d=identicon&s=25 James Britt (Guest)
on 2006-04-06 06:15
(Received via mailing list)
Jeff Cohen 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 Britt

"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
A402df36168b81b31c17adcbb5ae8cf4?d=identicon&s=25 Pistos Christou (pistos)
on 2006-04-06 17:57
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 Cohen 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.  :)  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
Cb6135c82400eadbc936f4cb336e3b9f?d=identicon&s=25 Jean-Charles Carelli (Guest)
on 2006-04-06 20:51
(Received via mailing list)
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 Fowler 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 Fowler 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
D812408537ac3a0fa2fec96eb8811559?d=identicon&s=25 John Carter (Guest)
on 2006-04-06 23:18
(Received via mailing list)
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, :-) 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 Carter                             Phone : (64)(3) 358 6639
Tait Electronics                        Fax   : (64)(3) 359 4632
PO Box 1645 Christchurch                Email : john.carter@tait.co.nz
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.
A9c4658e9e475e13d790ae419acf01b6?d=identicon&s=25 Simon Kröger (Guest)
on 2006-04-07 00:28
(Received via mailing list)
John Carter:
> 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
F0223b1193ecc3a935ce41a1edd72e42?d=identicon&s=25 zdennis (Guest)
on 2006-04-07 13:32
(Received via mailing list)
-----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 Cohen on the refactoring if (as James Britt points
out) you find yourself repeating this code in anyway. I also
agree with Benjohn Barnes 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-----
Ce8b03e5750097942c58e12b46724312?d=identicon&s=25 Giles Bowkett (Guest)
on 2006-04-07 18:45
(Received via mailing list)
On 4/5/06, Ryan Bates <rbates@artbeats.com> wrote:
> >
> > I find this helps to make code highly self documenting in many cases.
>
> I prefer that method as well. IIRC, Martin Fowler 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.
This topic is locked and can not be replied to.