[SUMMARY] Code Cleaning (#26)

R

Ruby Quiz

No takers for this idea, eh? We seem to like uglying up code better than
cleaning it! Well, I'm a firm believer in eating my own dog food, so...

Solving this quiz isn't really about the end result. It's more the process
involved. Here's a stroll through my process for the first script.

Timothy Byrd asked the right first question on Ruby Talk, which basically
amounts to, "What does this sucker do?" The programs used are semi-famous and
if you follow Redhanded, you probably already know:

http://redhanded.hobix.com/bits/batsmansFiveLineWiki.html

If you didn't, the -rcgi in the first line is a really big hint. -r is the
command-line short cut for a requiring a library, in this case cgi. From there,
it's pretty easy to assume that the script is a CGI script and that told me I
needed to get it behind a Web server to play with it.

I could have put it behind Apache and worked with it that way, but I chose to
use Ruby's standard WEBrick server instead. I'm glad I did too, because I ran
into a few issues while getting it running that were super easy to see, by
watching WEBrick's responses in my terminal. Here's the WEBrick script I wrote
to serve it up:

#!/usr/local/bin/ruby

require "webrick"

server = WEBrick::HTTPServer.new( :port => 8080,
:DocumentRoot => "cgi-bin" )

['INT', 'TERM'].each do |signal|
trap(signal) { server.shutdown }
end
server.start

That's super basic WEBrick in action. Pull in the library, initialize a server
with a port and document directory, set signal handlers for shutting down, and
start it up. This server can handle HTML, ERb templates, and, most importantly
here, CGI. Perfect.

I created the referenced "cgi-bin" directory right next to my server.rb script
and dropped in a file with the code to test, named "wiki.rb". I browsed over to
http://localhost:8080/wiki.rb and got to watch all my clever work go up in
flames. Luckily, bug hunting was pretty easy by watching WEBrick's output:

ERROR CGIHandler: /Users/james/Desktop/cgi-bin/wiki.cgi:
/usr/lib/ruby/1.6/cgi.rb:259:in `escapeHTML': private method `gsub'
called for []:Array (NameError)
from /Users/james/Desktop/cgi-bin/wiki.cgi:3

Okay, the error message isn't perfect, but it did get me thinking. Wasn't CGI's
handling of parameters changed somewhere around Ruby 1.8? A quick test with the
constant RUBY_VERSION did show that I was running an old version of Ruby
(1.6.8). Changing the shebang line got me back in business:

#!/usr/local/bin/ruby -rcgi

And I was greeted by a Wiki HomePage. Nifty. Now that I had it running, I felt
like I could start dealing with the code and see what it was doing.

The first thing I like to do with any code I can't read is to inject a lot of
whitespace. It helps me identify the sections of code. A cool trick to get
started with this in golfed/obfuscated Ruby code is a global find and replace of
";" with "\n". Then season with space, tab and return to taste. Here's my
space-out version:

#!/usr/local/bin/ruby -rcgi

H, B = %w'HomePage w7.cgi?n=%s'

c = CGI.new 'html4'

n, d = c['n'] != '' ? c['n'] : H, c['d']

t = `cat #{n}`

d != '' && `echo #{t = CGI.escapeHTML(d)} > #{n}`

c.instance_eval {
out {
h1 { n } +
a(B % H) { H } +
pre { t.gsub(/([A-Z]\w+){2}/) { a(B % $&) { $& } } } +
form("get") {
textarea('d') { t } +
hidden('n', n) +
submit
}
}
}

Now we're getting somewhere. I can see what's going on. This silly little
change opened my eyes to another problem immediately. Take a look at that
second line:

H, B = %w'HomePage w7.cgi?n=%s'

I now know what the original script was called: "w7.cgi". (The seventh Wiki?
Batsman is an animal!) I modified the line to play nice with my version:

H, B = %w'HomePage wiki.cgi?n=%s'

On to the next step. Let's clean up some of the language constructs used here.
We can spell out -rcgi, make those assignments slightly more obvious, eliminate
the ternary operator, clarify the use of the && operator, remove the dependancy
on the ugly $& variable, and swap a few { ... } pairs to do ... end pairs. I
thought about removing the instance_eval() call, but to be honest I like that
better than typing "c." 10 times. Let's see how the code looks now:

#!/usr/local/bin/ruby

require 'cgi'

H = 'HomePage'
B = 'wiki.cgi?n=%s'

c = CGI.new 'html4'

n = if c['n'] == '' then H else c['n'] end
d = c['d']

t = `cat #{n}`

`echo #{t = CGI.escapeHTML(d)} > #{n}` unless d == ''

c.instance_eval do
out do
h1 { n } +
a(B % H) { H } +
pre do
t.gsub(/([A-Z]\w+){2}/) { |match| a(B % match) { match } }
end +
form("get") do
textarea('d') { t } +
hidden('n', n) +
submit
end
end
end

The whole time I'm working with this code, I'm running it in my WEBrick server,
checking my changes and learning more about how it functions. One thing I'm
noticing is an occasional usage statement from cat:

cat: HomePage: No such file or directory

Sometimes it's being called on files that don't exist, probably before we add
content to a given Wiki page. It still works (returning no content), but we can
silence the warning. In fact, we should just remove the external dependancies
all together, making the code more portable in the process:

#!/usr/local/bin/ruby

require 'cgi'

H = 'HomePage'
B = 'wiki.cgi?n=%s'

c = CGI.new 'html4'

n = if c['n'] == '' then H else c['n'] end
d = c['d']

t = File.read(n) rescue t = ''

unless d == ''
t = CGI.escapeHTML(d)
File.open(n, "w") { |f| f.write t }
end

c.instance_eval do
out do
h1 { n } +
a(B % H) { H } +
pre do
t.gsub(/([A-Z]\w+){2}/) { |match| a(B % match) { match } }
end +
form("get") do
textarea('d') { t } +
hidden('n', n) +
submit
end
end
end

At this point, I understand the code well enough to extend the variable names
and add some comments, which should make its function pretty clear to others:

#!/usr/local/bin/ruby

# wiki.cgi

require 'cgi'

HOME = 'HomePage'
LINK = 'wiki.cgi?name=%s'

query = CGI.new 'html4'

# fetch query data
page_name = if query['name'] == '' then HOME else query['name'] end
page_changes = query['changes']

# fetch file content for this page, unless it's a new page
content = File.read(page_name) rescue content = ''

# save page changes, if needed
unless page_changes == ''
content = CGI.escapeHTML(page_changes)
File.open(page_name, 'w') { |f| f.write content }
end

# output requested page
query.instance_eval do
out do
h1 { page_name } +
a(LINK % HOME) { HOME } +
pre do # content area
content.gsub(/([A-Z]\w+){2}/) do |match|
a(LINK % match) { match }
end
end +
form('get') do # update from
textarea('changes') { content } +
hidden('name', page_name) +
submit
end
end
end

That's probably as far as I would take that code, without trying to make any
fundamental changes. The functionality is still pretty much the same (including
limitations!), but it's much easier to follow how the code works.

I used pretty much the same process to decrypt Florian's code, so I won't bore
you with a repeat. One additional tip that did help me through the complex
renamings is worth mentioning here though. When you need to rename a much-used
method or variable, just do it and try to compile. That will often give you the
exact line numbers that need updating.

One more interesting tidbit. When I entered the International Obfuscated Ruby
Code Contest, I used pretty much the opposite approach. I wrote a clean version
of my Ruby Quiz Loader, save that I tried to be a little more terse than usual.
Once I had that working, I just kept beating on it with the ugly stick until I
couldn't read it any more. For the curious, here's the original script:

#!/usr/bin/env ruby

require "open-uri"

puts "\nLoading...\n\n"

u, m, a = "http://www.rubyquiz.com/",
{ "nbsp" => " ", "lt" => :<, "gt" => :>, "amp" => :& },
[[/^\s+<\/div>.+/m, ""], [/^\s+/, ""], [/\n/, "\n\n"],
[/<br \/>/, "\n"], [/<hr \/>/, "-=" * 40], [/<[^>]+>/, ""],
[/^ruby/, ""], [/\n{3,}/, "\n\n"]]

open(u) { |w|
$F = w.read.scan(/li>.+?"([^"]+)..([^<]+)/)
}

puts "Ruby Quiz\n\n"
$F.each { |e|
i = e[0][/\d+/]
s = "%2s. %s" % [i, e[1]]
i.to_i % 2 == 0 ? puts(s) : print("%-38s " % s)
}

print "\n? "
n = gets.chomp.to_i

puts "\nLoading...\n\n"

open(u + $F[n-1][0]) { |q|
$_ = q.read[/^\s+<span.+/m]
a.each { |(s, r)| gsub!(s, r) }
gsub!(/&(\w+);/) { |e| m.key?($1) ? m[$1] : e }
while $_ =~ /([^\n]{81,})/
s = $1.dup
r = $1.dup
r[r.rindex(" ", 80), 1] = "\n"
r.sub!(/\n[ \t]+/, "\n")
sub!(/#{Regexp.escape(s)}/, r)
end
}

while sub!(/^(?:[^\n]*\n){20}/, "")
puts "#$&\n--MORE--"
g = $_
gets
exit if $_[0] == ?q
$_ = g
end
puts $_

That became:

http://iorcc.dyndns.org/2005/entries/IORCC-2005-03-21-01004-1

Tomorrow, we're back to normal Ruby Quiz material, this time from Jason
Bailey...
 
G

Gavin Kistner

No takers for this idea, eh? We seem to like uglying up code better
than
cleaning it! Well, I'm a firm believer in eating my own dog food,
so...

Thanks very much for this summary. I didn't participate in the quiz,
but I did learn a LOT just reading this summary. Both from a code point
of view as well as insight into a particular way of tackling this type
of problem.

Even if nobody posted a solution (even if nobody tried to clean up the
code but you), I'd still say this quiz was a success from the summary
alone :)
 
J

James Edward Gray II

Thanks very much for this summary. I didn't participate in the quiz,
but I did learn a LOT just reading this summary. Both from a code
point of view as well as insight into a particular way of tackling
this type of problem.

Even if nobody posted a solution (even if nobody tried to clean up the
code but you), I'd still say this quiz was a success from the summary
alone :)

Thanks Gavin. That's always been my number one goal for Ruby Quiz: To
provide a useful resource for the community.

For what it's worth, I do think code cleaning is a valuable exercise.
I enjoyed solving this quiz and learned a few tricks myself. If and
when the IORCC teams gets around to hosting the opposite contest, I
strongly encourage everyone to give it a shot.

James Edward Gray II
 
W

why the lucky stiff

Ruby said:
Solving this quiz isn't really about the end result. It's more the process
involved. Here's a stroll through my process for the first script.
I'm with Gavin. Your cleanups were great, your summary was engaging.
You're building volumes of some of the most enticing code. Sheer
stockpiling.

_why
 
D

Dave Burt

Ruby Quiz said:
No takers for this idea, eh? We seem to like uglying up code better than
cleaning it! Well, I'm a firm believer in eating my own dog food, so...

The quiz didn't make it to usenet. But Timothy Byrd's response and the
subsequent conversation did.

It's disappointing for me that I didn't find the time to do this quiz - an
interesting one indeed.

And maybe, if it would expiate me for my golfing, I should do it anyway when
I have the chance.

Cheers,
Dave
 
J

James Edward Gray II

The quiz didn't make it to usenet. But Timothy Byrd's response and the
subsequent conversation did.

It didn't? Sorry, I didn't know that.

Are we having more trouble with the gateway?

I just went to Google Groups and I do see it there. Also, you can
always check http://rubyquiz.com/. Quizzes are added to the page
automatically at the same time they are sent to the list, so it stays
current.

Just FYI.

James Edward Gray II
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads


Members online

No members online now.

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,007
Latest member
obedient dusk

Latest Threads

Top