From: Kevin Smith <sent@...>
Date: 2001-03-24T13:48:57+09:00
Subject: [ruby-talk:13163] Re: Amusing contrast

Dave Thomas wrote:
>>   File.foreach($config || "config.status") do |$_|
>>     next if /^#/
>>     if /^s%@(\w+)@%(.*)%g/
>>       name = $1
>>       val = $2 || ""
>>       next if /^(INSTALL|DEFS|configure_input|srcdir)$/ =~ name
>>       val.gsub!(/\$\{([^{}]+)\}/) { "$(#{$1})" }
>>       CONFIG[name] = val
>>     end
>>   end
>
>FWIW, I agree that's an ugly piece of code. So, here's a challenge for
>anyone who wants to try it. Re-express this code in 12 or fewer lines
>of Ruby. Make it look 'nice'. Make it more readable. Do it in
>different styles. And post the results here.

Well, I tried, but the code is so hard (for me) 
to understand that I couldn't figure out what the 
heck it is supposed to be doing. So I'll just 
share my goals if I were to clean up this code:

1. Make it longer than 12 lines. Well, that's not 
a hard requirement, but I sure can't think of any 
reason to limit it to 12 lines! Are we targeting 
this to run inside a toaster in less than 16K of 
RAM?

2. Put parens around all if conditions. Use 
do/end instead of {  } for the second block, and 
spread the second block over three lines. 
Whitespace is your friend.

3. Give meaningful names to /^s%@(\w+)@%(.*)%g/ 
and /\$\{([^{}]+)\}/ and 
/^(INSTALL|DEFS|configure_input|srcdir)$/ by 
assigning them to variables or constants.

4. Don't use the sneaky ($config || 
"config.status"). Same for the val= line. Instead 
say something obvious like:
  filemask = $config
  if(!filemask) filemask = "config.status"

5. I hate the "trailing if" syntax, so I would 
replace both next if... lines with if(...) next. 
This is probably the most personal item, and the 
most optional in my mind.

6. I wouldn't use $_ as the block variable. Using 
a global in that position is really jarring to 
those of us who are not Perl/regex gurus. I'd use 
"filename" or something similar.

I think that's about it. Otherwise, it looks 
great :-)

Kevin