Re: rubynuby code critique request

I may be a bit off here, but this project sounds very similar to one I've
been working on for some time.  I just recently rewrote it in ruby, which
was my first major experience with ruby.
You may be interested in taking a look, either for ideas, or (I hope ;))
if you want to help. :)

http://www.rubyforge.org/projects/rcoaster

I'm expecting to make another release, with XOSD support, sometime in the
next couple weeks.

On Mon, 8 Dec 2003, Robert Klemme wrote:

>
>
> "Jeff Dickens" <[e-mail]> schrieb im
> Newsbeitrag
> news:aVaAb.156105$[e-mail]...
> > I tried using the wiki at rubygarden.org but it just made a mess...
> probably
> > not designed for things this big.
> >
> > Anyhow, here's a (longish) program I've written as my first "real" ruby
> > program.  I'm reading from the "Programming Ruby" book at
> rubycentral.com,
> > and what do you know, I actually do have a jukebox of sorts.  Actually
> it's
> > a radio station I do some work for.  So I expanded the example code
> some.
> > Have a look and poke some holes.
> >
> > It reads a file that looks like this:
> >
> >   18 And Life; Skid Row
> >   19th Nervous Breakdown; Rolling Stones
> >   2000 Light Years From Home; Rolling Stones
> >   20th Centry Fox; Doors
> >   2112 (Parts 1-7); Rush
> >   29 Palms; Robert Plant
> >   30 Days In The Hole; Humble Pie
> >   5:15; Who
> >   867-5309; Tommy Tutone
> >   #9 Dream; John Lennon
> >   Abacab; Genesis
> >   Adam Raised a Cain; Bruce Springsteen
> >   A Day In The Life; Beatles
> >   A Face In The Crowd; Tom Petty
> >   After Midnight; Eric Clapton
> >   After The Gold Rush; Neil Young
> >   Against The Wind; Bob Seger
> >   A Hard Day's Night; Beatles
> >
> >
> > It reads the file and can create a word list that could be used for a
> > full-text search, a listing by artist, and a listing by artist in HTML
> with
> > a clickable index.
> >
> > Here's the code.  Your input is appreciated.
> >
> > #!/usr/local/bin/ruby -w
> >
> > require 'optparse'
> >
> >
> > class SongList
> >   def initialize
> >     @songs = Array.new
> >     @index = WordIndex.new
> >     @aindex = ArtistIndex.new
> >   end
> >   def append(aSong)
>
> Formatting the names is not a responsibility of the list.  This should be
> done either in the Song or during reading.
>
> >     aSong.name.sub!(/\s+$/,'')    # lose leading and trailing
> >     aSong.artist.sub!(/\s+$/,'')  # whitespace
> >     aSong.name.sub!(/^\s+/,'')
> >     aSong.artist.sub!(/^\s+/,'')
>
>
> >     @songs.push(aSong)
> >     @index.index(aSong, aSong.name, aSong.artist)
> >     @aindex.aindex(aSong, aSong.artist)
> >     self
> >   end
> >   def [](key)
> >     return @songs[key] if key.kind_of?(Integer)
> >     return @songs.find { |aSong| aSong.name == key }
> >   end
>
> Can be delegated to the indexes:
> >   def lookup(aWord)
> >     @index.lookup(aWord)
> >   end
> >   def wordlist
> >     @index.wordlist
> >   end
>
> >   def wordindex(outFile)
> >     @index.wordlist.each do |aWord|
> >       outFile.puts aWord
> >       @index.lookup(aWord).each do |aSong|
> >         outFile.puts "\t" + aSong.name + "; " + aSong.artist
> >       end
> >     end
> >   end
>
> Can be delegated to the indexes:
> >   def alookup(artist)
> >     @aindex.alookup(artist)
> >   end
> >   def listartists
> >     @aindex.listartists
> >   end
>
> >   def listsongs(outFile)
> >     @aindex.listartists.each do |anArtist|
> >       outFile.puts anArtist
> >       @aindex.alookup(anArtist).sort \
> >       {|a,b| a.name <=> b.name}.each do |aSong|
> >         outFile.puts "\t" + aSong.name
> >       end
> >     end
> >   end
>
> I'd put the HTML methods into antoher class because this is view specific,
> i.e. you might want to do other presentations as well:
> >   def html(outFile)
> >     outFile.print "<a name=\"top\" id=\"top\"></a>\n"
> >     @aindex.listartists.each do |anArtist|
> >       safeKey = anArtist.gsub(/[\s\.\-\,\/\\\']/, '')
> >       outFile.print "<a href=\"##{safeKey}\">"
> >       outFile.print "<div class=\"artistx\">#{anArtist}</div></a>\n"
> >     end
> >     @aindex.listartists.each do |anArtist|
> >       safeKey = anArtist.gsub(/[\s\.\-\,\/\\\']/, '')
> >       htmlDetail(outFile, anArtist, safeKey)
> >     end
> >   end
> >
> >   def htmlDetail(outFile, anArtist, safeKey)
> >     outFile.print "<div class=\"artist\">"
> >     outFile.print "<a name=\"#{safeKey}\" id=\"#{safeKey}\">"
> >     outFile.print "#{anArtist}</a>\n"
> >     outFile.print "<a href=\"#top\" class=\"toplink\">top</a></div>\n"
> >     @aindex.alookup(anArtist).sort \
> >     {|a,b| a.name <=> b.name}.each do |aSong|
> >       outFile.print "\t<div class=\"song\">"
> >       outFile.print "#{aSong.name}</div>\n"
> >     end
> >   end
> >
> > end #class SongList
>
> Index classes are not needed when using Hash's features:
> > class WordIndex
> >   def initialize
> >     @index = Hash.new(nil)
> >   end
> >   def index(anObject, *phrases)
> >     phrases.each do |aPhrase|
> >       aPhrase.scan(/\w[-\w]+/) do |aWord|   # extract each word
> >         aWord.downcase!
> >         @index[aWord] = [] if @index[aWord].nil?
> >         @index[aWord].push(anObject)
> >       end
> >     end
> >   end
> >   def lookup(aWord)
> >     @index[aWord.downcase]
> >   end
> >   def wordlist
> >     @index.keys.sort
> >   end
> > end
> >
> > class ArtistIndex
> >   def initialize
> >     @aindex = Hash.new(nil)
> >   end
> >   def aindex (anObject, artist)
> >     @aindex[artist] = [] if @aindex[artist].nil?
> >     @aindex[artist].push(anObject)
> >   end
> >   def alookup(artist)
> >     @aindex[artist]
> >   end
> >   def listartists
> >     @aindex.keys.sort
> >   end
> > end
> >
>
> Ok.
> > class Song
> >   def initialize(name, artist)
> >     @name = name
> >     @artist = artist
> >   end
> >   attr_reader :name, :artist
> >   def to_s
> >     "Song: #{@name}--#{@artist}"
> >   end
> > end
> >
> > def build_data(inFile); # Read the data and build structures
>
> # don't use a global var here, just return the SongList
> >   $songs = SongList.new # inFile is an open file descriptor
> >   inFile.each do |line|
> >     name, artist = line.chomp.split(/\s*;\s*/)
> >     unless name and artist
> >       puts "input data format error"
>
> better use an exception here and don't terminate the whole script:
> >       exit
> >     end
> >     $songs.append Song.new(name, artist)
> >   end
> > end
> >
> >
> > #---------------Main Begins Here--------------------------
> >
> >
> > $0 = File.basename($0)
> >
> > vars = {}
> > ARGV.options do
> >   |opts|
> >   opts.banner = "\nUsage: #{$0} [options] [input filename]\n\n" \
> >   "Reads from stdin if [input filename] is \n" \
> >   "not present.\n\n" \
> >   "Data format is \"Song Title; Artist\"\n\n"
> >
> >   opts.on_head("Options:")
> >
> >   opts.on("-w", "--wordindex WORDINDEXFILE", String,
> >    "Generate Word Index",
> >    "in WORDINDEXFILE") {|vars[:wordIndexFile]|}
> >
> >   opts.on("-g", "--generatehtml HTMLFILE", String,
> >    "Generate HTML File",
> >    "in HTMLFILE") {|vars[:htmlFile]|}
> >
> >   opts.on("-l", "--list LISTFILE", String,
> >    "Generate List File",
> >    "in LISTFILE") {|vars[:listFile]|}
> >
> >   opts.on_tail("-h", "--help", "show this message") {puts opts; exit}
> >
> >   opts.parse!
> >   end
> >
> > if vars.length < 1
> >   puts "Nothing to do"
> >   puts ARGV.options
> >   exit
> > end
> >
> > if ARGV.length > 1
> >   puts "Too many arguments"
> >   exit
> > end
> >
> > if ARGV.length == 1
> >   then
> >   unless FileTest.readable?(ARGV[0]) # test if input file exists
> >     puts ARGV[0] + " not readable"
> >     exit
> >   end
> >   inFile=File.open(ARGV[0])  # open inputfile
> >   build_data(inFile)  # and read/build data
> >   inFile.close
> >   else
> >   build_data($stdin)  # just use stdin if no input file
> > end
> >
> > if vars[:wordIndexFile]  # Generate word index file if -w
>
> better use block form, it's safer:
> >   outFile = File.new(vars[:wordIndexFile], "w")
> >   $songs.wordindex(outFile)
> >   outFile.close
>
> File.open(vars[:wordIndexFile], "w") do |outFile|
>   $songs.wordindex(outFile)
> end
>
> > end
> >
> > if vars[:listFile]  # Generate listing file if -l
> >   outFile = File.new(vars[:listFile], "w")
> >   $songs.listsongs(outFile)
> >   outFile.close
> > end
> >
> > if vars[:htmlFile]  # Generate html file if -g
> >   outFile = File.new(vars[:htmlFile], "w")
> >   $songs.html(outFile)
> >   outFile.close
> > end
>
> That's how I would start as a quick solution.  Disadvantages:
>
>  - it's not complete
>
> require 'set'
>
> # special class to prevent lookups from creating entries
> class Index < Hash
>   def insert(key, value)
>     ( self[key] ||= Set.new ) << value
>   end
> end
>
> class Song
>   attr_accessor :artist, :title
>
>   def initialize(title, artist)
>     @artist, @title = artist, title
>   end
>
>   def eql_impl?( s )
>     title == s.sitle && artist == s.artist
>   end
>
>   alias :eql? :eql_impl?
>   alias :== :eql_impl?
>
>   def hash
>     title.hash ^ artist.hash
>   end
>
>   private :eql_impl?
> end
>
>
> class SongList
>   def create_index
>     # Hash.new {|h,k|h[k]=Set.new}
>     Index.new
>   end
>
>   attr_reader :artist_index, :word_index, :songs
>
>   def initialize
>     @artist_index = create_index
>     @word_index = create_index
>     @songs = Set.new
>   end
>
>   def add(song)
>     songs << song
>     artist_index.insert( song.artist, song )
>
>     [song.artist, song.title].each do |str|
>       str.scan( /\w+/ ) {|word| word_index.insert( word, song ) }
>     end
>   end
>
>   def load(file)
>     File.open( file ) do |io|
>       io.each_line do |line|
>         if /^\s*\b(.*)\b\s*;\s*\b(.*)\b\s*$/ =~ line
>           add Song.new($1, $2)
>         end
>       end
>     end
>   end
>
>   def self.load(file)
>     sl = self.new
>     sl.load( file )
>     sl
>   end
> end
>
> # simple test
> sl = SongList.load( "songs.txt" )
> p sl
> p sl.word_index[ "A" ]
>
>
> Kind regards
>
>     robert
>
>

Derek Lewis

===================================================================
      Java Web-Application Developer

      Email    : [e-mail]
      Cellular : 604.312.2846
      Website  : http://www.lewisd.com

 "If you've got a 5000-line JSP page that has "all in one" support
 for three input forms and four follow-up screens, all controlled
 by "if" statements in scriptlets, well ... please don't show it
 to me :-).  Its almost dinner time, and I don't want to lose my
 appetite :-)."
      - Craig R. McClanahan