[lnkForumImage]
TotalShareware - Download Free Software
Confronta i prezzi di migliaia di prodotti.
Asp Forum
 Home | Login | Register | Search 


 

Forums >

comp.lang.ruby

Beautiful Code : Pity he didn't ask here...

John Carter

7/10/2007 12:15:00 AM

40 Answers

James Gray

7/10/2007 12:53:00 AM

0

On Jul 9, 2007, at 7:15 PM, John Carter wrote:

> Well, maybe for the 2nd edition, heres my bash at cleaning it up...
>
> For example scanning a log file.....
>
> EXAMPLE 4-2 . Printing article names
> 1 ARGF.each_line do |line|
> 2 if line =~ %r{GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/\d\d/
> [^ .]+) }
> 3 puts $1
> 4 end
> 5 end
>
> Since most Linux distro's roll the log files to a reasonable size I
> would have just gone with...
>
> ARGF.read.scan( %r{GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/\d\d/
> [^ .]+) }) do |match|
> puts $1
> end

Hmm, I don't think we should slurp when we don't need too. There's
no reason to be wasteful.

I really liked the rest of your fixes though.

James Edward Gray II

Ryan Davis

7/10/2007 6:56:00 AM

0


On Jul 9, 2007, at 17:53 , James Edward Gray II wrote:

>> ARGF.read.scan( %r{GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/\d\d/
>> [^ .]+) }) do |match|
>> puts $1
>> end
>
> Hmm, I don't think we should slurp when we don't need too. There's
> no reason to be wasteful.

Actually there is plenty of reason to be wasteful:

1) We can.
2) It leads to code that reads like the above.
3) We can!
4) It is only a short hop to:

puts ARGF.read.scan( %r%GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/
\d\d/[^ .]+) %).join("\n")

5) We can!!
6) It is our moral obligation to use our oft-idle and underused
computers to their fullest.

There was a 7th... but I forgot what it was.

Martin DeMello

7/10/2007 7:58:00 AM

0

On 7/10/07, John Carter <john.carter@tait.co.nz> wrote:
> Since most Linux distro's roll the log files to a reasonable size I would have just gone with...
>
> ARGF.read.scan( %r{GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/\d\d/[^ .]+) }) do |match|
> puts $1
> end

ARGF.read.scan( %r{GET /ongoing/When/\d{3}x/(\d#{4}/\d#{4}/[^ .]+) }) do |match|
puts $1
end

martin

Robert Dober

7/10/2007 9:48:00 AM

0

On 7/10/07, Martin DeMello <martindemello@gmail.com> wrote:
> On 7/10/07, John Carter <john.carter@tait.co.nz> wrote:
> > Since most Linux distro's roll the log files to a reasonable size I would have just gone with...
> >
> > ARGF.read.scan( %r{GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/\d\d/[^ .]+) }) do |match|
> > puts $1
> > end
>
I am with James do not slurp in the whole string it just does not make
sense to me. I would feel very uneasy using regexps on large strings
even if right now there seems no backtracking issues to be present,
but code evolves sometimes ;)

> ARGF.read.scan( %r{GET /ongoing/When/\d{3}x/(\d#{4}/\d#{4}/[^ .]+) }) do |match|

Nope Martin read the original code again!! All the / and \ ;).
I was about to fall into the same trap.
As a matter of fact I miss all beauty in that code :( The regexp is
just ugly, some ideas for cosmetic surgery ;)

def dig_dirs *args
Regexp.new args.map{|n| "\d" * n}.join("/")
end

puts ARGF.map { |line|
line =~ %r{ GET /ongoing/When/\d{3}x/(#{dig_dirs 4, 2, 2}/[^ .])}
$1
}.compact

But beauty lies to everybody (or was that "in the eyes of the beholder"? ).

Cheers
Robert

--
I always knew that one day Smalltalk would replace Java.
I just didn't know it would be called Ruby
-- Kent Beck

Robert Dober

7/10/2007 10:09:00 AM

0

On 7/10/07, John Carter <john.carter@tait.co.nz> wrote:
> Just got the O'Reilly announcement of the book "Beautiful
<snip>
> In Example 4.3 he has...
> 1 counts = {}
> 2 counts.default = 0

>
> Personally I prefer...
> counts = Hash.new(0)
> or
> counts = Hash.new{|hash,key| hash[key]=0}
> buts that's personal preference I guess.
>
> In example 4.4 he rightly identifies line 10 as a little ugly and bemoans the lack of sort_by_value in hash.
> 10 keys_by_count = counts.keys.sort { |a, b| counts[b] <=> counts[a] }
> it seems he doesn't know about..
>
> count.keys.sort_by{|key| count[key]}
>
> Of course he could have done...
>
> class Hash
> def sort_keys_by_value
> keys.sort_by{|key| fetch(key)}
> end
> end
or

tops = counts.sort_by{|*kv| kv.last}.reverse[0..9]
puts tops.map{ |k,v| "#{v}: #{k}"}

#each is just a primitive for Enumerables, right?

>
>
> Example 4.5 looks like a poster child for the Hash.new block approach...
> 4 @hash = {}
> .
> .
>
> 9 if @hash[s[0]]
> 10 @hash[s[0]] << [ s[1], article ]
> 11 else
> 12 @hash[s[0]] = [ s[1], article ]
> 13 end
>
>
> Replace that with...
>
> 4 @hash = Hash.new{|hash,key| hash[key] = []}
>
> 10 @hash[s[0]] << [ s[1], article ]
>
>
> That makes it real pretty code.

Save for

File.open(some_name).each_line

Let the GC close the file!? It is not going to happen !!

<snip>
Robert

--
I always knew that one day Smalltalk would replace Java.
I just didn't know it would be called Ruby
-- Kent Beck

Daniel Martin

7/10/2007 12:12:00 PM

0

John Carter <john.carter@tait.co.nz> writes:

> Just got the O'Reilly announcement of the book "Beautiful
> Code"... here is the sample chapter by Tim Bray on Ruby!
>
...
>
> For example scanning a log file.....
>
> Since most Linux distro's roll the log files to a reasonable size I would have just gone with...
>
> ARGF.read.scan( %r{GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/\d\d/[^ .]+) }) do |match|
> puts $1
> end

As James said, I'm going to have to disagree on this, though I'd clean
it up to:

article_pattern = %r{GET /ongoing/When/\d\d\dx/(\d\d\d\d/\d\d/\d\d/[^ .]+) }
ARGF.each_line do |line|
line.scan(article_pattern) do |article,|
puts article
end
end

Look, no global variables!

I agree with your use of Hash.new(0), but disagree that it's mere
personal preference - it's much more common to use even the single
argument form in the constructor than to use default=.

It is odd that he seems unaware of Array.sort_by - anyone know when
this was added to Ruby core? The book could be rather out of date.

> Example 4.5 looks like a poster child for the Hash.new block approach...

Agreed. I wonder if perhaps he's spent more time in python or some
other dynamic language than Ruby.

> In fact, I can't find any pretty reason to have the class BigHash at
> all....looks like his Java roots are showing through.

Yeah; in Java, you can't have code existing outside of a class, and I
suspect that's a big part of it.

> On the other hand both have the classic bug that (high + low) or
> (l+u) overflows on large arrays...

Actually, his code doesn't: java arrays can only have 2**31 entries
(since their index is a java int), and java ints are always signed.
Therefore, any overflow in the addition in java yields a negative
integer, and >>> in java does right shift without sign extension.
(That is, with 0s shifted in at the left).

The glibc code does have that problem, but only on arrays that have
more than (maximum size_t)/2 elements.

--
s=%q( Daniel Martin -- martin@snowplow.org
puts "s=%q(#{s})",s.to_a.last )
puts "s=%q(#{s})",s.to_a.last

dblack

7/10/2007 12:12:00 PM

0

Christoffer Sawicki

7/10/2007 12:28:00 PM

0

Hello.

On 7/10/07, John Carter <john.carter@tait.co.nz> wrote:
> count.keys.sort_by{|key| count[key]}

IMHO, the most elegant way to do this is:

hash.sort_by { |k, v| v }.map { |k, v| k }

...where k = key and v = value.

Cheers,

--
Christoffer Sawicki
http://...

Gregory Brown

7/10/2007 1:14:00 PM

0

On 7/10/07, Wayne E. Seguin <wayneeseguin@gmail.com> wrote:
> On Jul 10, 2007, at 08:27 , Christoffer Sawicki wrote:
> > Hello.
> >
> > On 7/10/07, John Carter <john.carter@tait.co.nz> wrote:
> >> count.keys.sort_by{|key| count[key]}
> >
> > IMHO, the most elegant way to do this is:
> >
> > hash.sort_by { |k, v| v }.map { |k, v| k }
> >
> > ...where k = key and v = value.
> >
> > Cheers,
>
> Ack, pet peeve.
>
> hash.sort_by { | key, value | value } .map { | key, value | key }
>
> If you have to specify "where k = key and v = value" then these
> should have been used in your code.
>
> Always favor readability at the expense of verbosity, both your
> future self and whoever else maintains your code will thank you.

I find myself always using |k,v|, when used on something hashlike I
don't think readability suffers.

James Gray

7/10/2007 1:24:00 PM

0

On Jul 10, 2007, at 8:13 AM, Gregory Brown wrote:

> On 7/10/07, Wayne E. Seguin <wayneeseguin@gmail.com> wrote:
>> On Jul 10, 2007, at 08:27 , Christoffer Sawicki wrote:
>> > Hello.
>> >
>> > On 7/10/07, John Carter <john.carter@tait.co.nz> wrote:
>> >> count.keys.sort_by{|key| count[key]}
>> >
>> > IMHO, the most elegant way to do this is:
>> >
>> > hash.sort_by { |k, v| v }.map { |k, v| k }
>> >
>> > ...where k = key and v = value.
>> >
>> > Cheers,
>>
>> Ack, pet peeve.
>>
>> hash.sort_by { | key, value | value } .map { | key, value | key }
>>
>> If you have to specify "where k = key and v = value" then these
>> should have been used in your code.
>>
>> Always favor readability at the expense of verbosity, both your
>> future self and whoever else maintains your code will thank you.
>
> I find myself always using |k,v|, when used on something hashlike I
> don't think readability suffers.

I've also recently adopted the trick of using _ as an unused
parameter name. I believe it was Ara that first suggested this and I
think it's a great idea:

hash.sort_by { |key, _| … }…

James Edward Gray II