=?ISO-8859-1?Q?Aleksi_Niemel=E4?= <zak / ale.cx> wrote:
> On Sun, 28 Jan 2001, Brian F. Feldman wrote:
> 
> > <ale / crimson.propagation.net> wrote:
> > > Anyway, this bug doesn't bite too often by accident. If you adopt
> > > "right" style to write code (include operator or dot as a hint of
> > > continuation to the same line) it's very rare event you need
> > > '\'. During my sufficiently short Ruby career I can't remember using
> > > this notation more than two times.
> > > 
> > > Usually when a statement grows over line there's something in your
> > > code which uses it's last way of signalling it should be fixed.
> > 
> > I disagree with that, since it is in the style of functional programming to 
> > have a large "chain" of methods acting upon a series of lists (or, in Ruby's 
> > case, Arrays).  For example:
> > 
> >     def CDDB.from_sql(res)
> >         unsubber = lambda {|s|
> >             s.scan(/[^\\]("([^"\\]|\\.)*")/).
> >               collect {|e| e[0][1..-2].gsub(/\\(.)/, '\1')}
> >         }
> >         CDDBStruct.new(unsubber.call(res[0]), res[1], unsubber.call(res[2]),
> >           res[3], unsubber.call(res[4]),
> >           res[5][1..-2].split(',').collect {|x| x.to_i})
> >     end
> > 
> > You'll probably see even longer chains than that all the time, and in those 
> > cases I think that when a statement is more than a line because of this, 
> > it's specifically doing things right!
> 
> While I agree with you that there's a certain tendency to produce
> chains of calls, I have to keep staying behind my opinion. My very
> argument against this style of coding is the fact that it's painful to
> read above code, and even more painful to change it.
> 

You think so?  My eyes, when following it, are drawn to the periods and the 
matching parentheses/braces/brackets.  In other words, my ... style? ... 
will often include more punctuation whereas yours will include whitespace 
because that's what really helps you visually pick out control flow.

> I like call chaining, even to the point I really like to get away with
> most method! anomalities (returning nil sometimes; actually I fixed
> one of these from someone else's code which didn't work for me).

Example (evil one, maybe :) from the same file:

        subber = lambda {|s|
            s.gsub!(/(\\*)\\n/) {$1.length[0] == 1 ? $1 + "\n" : $&}
            s.gsub!(/(\\*)\\t/) {$1.length[0] == 1 ? $1 + "\t" : $&}
            s.gsub!(/\\\\/, '\\')
            s.gsub!(/'/, "''")
            s.gsub!(/[{}]/, '\\\\\0')
            s
        }

It's not actually chained because I decided to use the bang-methods.  
I think that because every line lines up the same, it's a very clear idiom 
for performing a series of operations.  I'm not sure which I prefer.

Also, I notice that I tend to give ephemeral arguments one-letter names.  
How shell-scripty!

> Simultaneously when you reorganize the above code there's an effect
> that you give name to a thing you handle; be it a command chain,
> a parameter or a helper variable.
> 
>      def CDDB.from_sql(res)
>          quoted_string_re = /[^\\]("([^"\\]|\\.)*")/
>          unsubber = lambda do |s|
>              s.scan(quoted_string_re).collect do |e| 
>                  e[0][1..-2].gsub(/\\(.)/, '\1')}
>              end
>          end
>          listed_numbers = res[5][1..-2].split(',').collect {|x| x.to_i}
>          CDDBStruct.new(unsubber.call(res[0]), res[1],
>                         unsubber.call(res[2]), res[3], 
>                         unsubber.call(res[4]), listed_numbers)
>      end
> 
> I'm not claiming this example is in some way the ultimate, most
> readable version, but I find my version easier to read and mess
> around.

I'm one of those people that believe that if you're only using something 
once, it's usually wrong to give it a name unless otherwise it would truly 
make things very hard to follow.  In this specific case, I usually /try/ to 
let my regexps speak for themselves.  If they're so hard to follow that I 
can't tell what they're for, they're probably not very good at all :)  For 
instance, key to this one is it's "something in quotes not preceded by a 
backslash consisting of a series of strings that are either not a quote, not 
a backslash, or a backslash followed by anything".  This regexp, to me, is 
an important part of the code, so it should be read and understood like any 
of the rest.

I agree that using listed_numbers probably does make things that much easier 
to follow for a lot of people.  I was brought up on the BSD KNF style in C, 
which dictates (8-space) tabs for indentation but  four spaces for statement 
continuation.  I carry this to Ruby by using (4-space) tabs and 2-space 
statement continuation indentation, like in the CDDBStruct.new() call.  This 
is just the way I've been "raised": make the whitespace always mean 
something when it's there and always be consistent.  Something I forgot 
about in this case is at least attempting to make the lines more uniform, so 
I'd personally want to use something more like:

        CDDBStruct.new(unsubber.call(res[0]),
	  res[1], unsubber.call(res[2]),
          res[3], unsubber.call(res[4]),
          res[5][1..-2].split(',').collect {|x| x.to_i})

Uniformity makes it a bit more pleasing, but you're right that it can't be 
as uniform as it could when the arguments are all converted to a form which 
make lining up more possible.  I could go either way.

> Still I'd like to exploit the possible rule of "unsub every second
> parameter", name the mystical range of 1..-2, give better names to
> variables in unsubber, maybe split the long line of listed_numbers
> definition into two lines and call collect! on the latter.

I don't think that's a very good rule, because it's really just... 
coincidental:

    CDDBStruct = Struct.new('CDDB',
      :discid, :dtitle, :ttitle, :extd, :extt, :playorder)

Stored in SQL, it's:

... freedb (discid text[], dtitle text, ttitle text[],
  extd text, extt text[], playorder int[]);

The range 1..-2 will probably be seen an awful lot as the most common idiom 
for taking the "inside" of a string.  I can't think of any way to express it 
better :)  I still feel not giving things names until the end of a chain is 
a good thing.  For the listed_numbers, it's "take the sixth result, without 
the first and last characters, tokenize it by commas, and convert the array 
of strings to an array of integers".

> Anyway, I guess you get my point. I think it's quite rare event that
> you really have to have over 75 character (or some limit) lines. I
> hear the exceeding line length as a cry requesting some code massage.
> 
> There are some simple cases where the change is easy to do, like
> spreading inlined blocks to multiple lines ({..} notation to do..end),
> and with parameter passing I feel no guilty having couple of lines
> between opposing parentheses (as long as the content doesn't contain
> too much of executed code).
> 
> Do we still disagree ?)

It's a matter of style :)  I feel you raise a lot of good points, but I find 
my ethos on it to be different from yours.  So, I agree to disagree on parts 
of it, but I agree with you when you suggest that making things more 
"documentary" is useful.  Perhaps I should use comments some time...

P.S.: I just noticed that using Ruby for this kind of stuff rather than C, 
saying nothing about the amount of code needed, I never need any comments in 
the Ruby code to go back and understand any of it, but for C I always need 
to comment all manner of uncomplicated code.

-- 
 Brian Fundakowski Feldman           \  FreeBSD: The Power to Serve!  /
 green / FreeBSD.org                    `------------------------------'