Best practices

Jean-Charles_Carelli · 5 April 2006 20:39

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

Keith_Sader · 5 April 2006 21:10

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.

···

On 4/5/06, Jean-Charles Carelli <jnchrls@flashcodersny.org> wrote:

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

--
Keith Sader
ksader@gmail.com
http://www.saderfamily.org/roller/page/ksader

Ara.T.Howard6 · 5 April 2006 21:34

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

···

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

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?

--
be kind whenever possible... it is always possible.
- h.h. the 14th dali lama

Benjohn_Barnes · 5 April 2006 21:40

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

···

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

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?

John_Carter · 6 April 2006 21:16

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))

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

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.

···

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

Zach_Dennis1 · 7 April 2006 11:31

Jean-Charles Carelli wrote:

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?

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

chiaro_scuro · 5 April 2006 21:27

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/

On 4/5/06, Keith Sader <ksader@gmail.com> wrote:

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.

On 4/5/06, Jean-Charles Carelli <jnchrls@flashcodersny.org> wrote:
> 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?

Ryan_Bates · 5 April 2006 23:01

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.

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

···

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

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

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?

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

James_Britt4 · 5 April 2006 23:20

...

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.

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.

···

ara.t.howard@noaa.gov wrote:

--
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

Simon_Kroger · 6 April 2006 22:26

John Carter:

[...]
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))

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

Jeff · 6 April 2006 01: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

···

--
Posted via http://www.ruby-forum.com/.

Giles_Bowkett · 7 April 2006 16:44

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.

···

On 4/5/06, Ryan Bates <rbates@artbeats.com> wrote:

On Apr 5, 2006, at 2:40 PM, Benjohn Barnes wrote:
> 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.

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.

--
Giles Bowkett
www.gilesgoatboy.org

James_Britt4 · 6 April 2006 04:12

Jeff Cohen wrote:

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

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

Pistos_Christou1 · 6 April 2006 15:58

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

···

--
Posted via http://www.ruby-forum.com/.

Jean-Charles_Carelli · 6 April 2006 18:48

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

···

On Apr 6, 2006, at 11:58 AM, Pistos Christou wrote:

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

--
Posted via http://www.ruby-forum.com/.