From: Phrogz <phrogz@...>
Date: 2010-04-19T02:50:15+09:00
Subject: Re: Elegant Solution to a Seemingly Simple Problem?

On Apr 18, 1:23 am, Derek Cannon <novelltermina...@gmail.com> wrote:
> [...]
> Earlier, someone on the forum showed me a very elegant way to collect
> this information (I use Nokogiri). It was:
>
> doc = Nokogiri::HTML(open(url))
>
> raw_course_list = doc.css("tr").collect { |row|
>   row.css("td").collect { |column|
>     column.text.strip
>   }
>
> }
> [...]
> This works perfectly, except in 3 main cases.
>
> *** Problem 1: The <tr> does not contain course information. (It's some
> irrelevant part of the HTML). In this case, I did the following:
> raw_course_data.reject! { |i| i.size != 4 }, would filtered out
> non-courses. Note: no tables without course data had the size of one
> with course data (in the non-simplified version, the size is actually
> much larger).
>
> So, already I think it's ugly coding! It firsts loads ALL <tr> contents
> into arrays, then rejects them after creation.
> [...]

Generalized, you have an array of values and you want to map a subset
of them to new array. There are (at least) four patterns you can use
to handle this sort of situation:

1) Map the unwanted elements to a 'broken' value and then reject the
broken values later. (What you are doing now.) This can be hard if you
don't have a way of creating a broken value. For example, you might be
mapping all values directly to an object, but you don't have enough
information for the object constructor and no way of making up clearly
spurious values. Further, it's inefficient as you do the work and use
the memory of creating the object only to throw it out later.

2) Map the unwanted elements to nil and compact the array afterwards.
In your case, you'd need to look at the TDs in your row and decide if
you wanted to map the row to the mapping of them or nil. This is
convenient in terms of one-liners, but still slightly inefficient
because you're creating an intermediary array packed with nils that
you don't want. (You should be clear, though, that computational
inefficiency is not always more important than programmer convenience
of code clarity.)

3) Instead of using map (or the same effect under the longer name
'collect', as Robert apparently likes) to create a new array from your
original, explicitly create the new array and push values only as
valid. This is basically the same as above, but without the nil values
and the later compact. For example:
  raw_course_list = []
  doc.css("tr").each { |row|
    tds = row.css("td")
    if tds.have_the_values_I_want
      raw_course_list << tds.map{ |col| ... }
    end
  }

4) Use map (collect) on the array as in #1 or #2, but before that do a
pass through your source array and sanitize it. Sanitization might be
mapping values to nil and then compacting (thus very similar to #2),
or fixing values (as in your TBA or continued description case). This
feels cleaner, but note that this has you doing one (or two, in the
case of map+compact) passes on your data before you get around to
mapping it.


Here's (very roughly) what I might do given what you wrote:
  # Assuming you're using Ruby 1.9
  course_info = []
  trs = doc.css('tr')
  trs.each.with_index{ |row,i|
    tds = row.css('td')
    title = ...
    prof  = ...
    days  = ...
    times = ...
    desc  = ...
    next_row = trs[i+1]
    if next_row && next_row.is_a_continuation?
      # Add content from next_row to description
      # If needed, invalidate next_row so it will be skipped
    elsif title && prof && days # If you have all the information you
need
      course_info << Course.new( title, prof, days )
    end
  }

Regardless of the approach you use, remember that even though you're
annoyed that you are 'processing' (in one form or another) invalid
entries, you have to touch every row to find out if you like it or
not. It's up to you for how you detect which are invalid and handle
them.