Meinrad said:
[...]
i like the article!
yeah, and even if code using the fields is coupled tightly to the
created classes, the solution is highly reusable.
-- henon
The point about coupling (mentioned in the second-to-last paragraph of
the article) is important, and I feel it is dismissed to easily in the
article. There are some tradeoffs to consider, though perhaps they are
out of the scope of the article, which is intended as an exercise, not
as a complete guide:
1. Suppose your code needs to _discover_ what fields are in the file?
You can use #instance_methods(false), but that is not perfect: you have
to filter out "to_s" and "inspect", which were added by #make. And what
if you add a new method in addition to the ones generated by #make? The
field names could be stored in a list kept in class instance variable...
2. Once you have discovered the field names, you have to use
send(fieldname) and send("#{fieldname}=") to access them. That's more
awkward and (at least in the second case) less efficient than Hash#[]
and #[]=. Who's the "second class citizen" in this case?
3. If you really know the field names "in advance" (that is, you have
enough information to hard code them into your program), rather than "by
discovery", then maybe it is better to use a different metaprogramming
style in which the fields are declared using class methods:
class Person
field :name, :favorite_ice_cream, ...
end
In this way, some rudimentary error checking can be performed when
reading the file, rather than failing later when trying to serve ice
cream to the person. (I just hate it when my ruby-scripted robo-fridge
serves me passion fruit and rabbit dropping ice cream.) This is not
always the best way to go (what if, as the article points out, fields
get added to the file?), but one more thing to keep in mind.
4. Is it always a good idea to couple the class name with the file name?
Maybe the class's identity should be associated with the set of fields
defined in the header? Why not _reuse_ the anonymous class if the header
is the same as those in some other file you imported earlier? (This
could be done using a hash mapping sorted lists of column names to
classes.) That would make it possible to use == to compare objects read
from different files. Further, it would let you use x.class == y.class
to determine if x and y came from files with compatible formats (same
fields, but maybe in a different order).
5. Maybe Struct would serve just as well, since it takes care of
everything in the class_eval block. For example:
klass = Struct.new(*names.map{|s|s.intern})
None of these are necessarily problems, depending on what you are trying
to do, but alternate solutions (for example, using hashes) are worth
considering. Metaprogramming is not always the best solution, though it
is good to have it in your pocket.
Some minor quibbles:
1. In DataRecord.make, if the file happens to be empty, data.gets.chomp
will raise an exception and the file will not be closed. Similarly in
the #read method of the generated class. Why not use a block with File.open?
2. The second way of referring to the class:
require 'my-csv'
DataRecord.make("people.txt") # Ignore the return value and
list = People.read # refer to the class by name.
should raise the hackles on a ruby programmer's neck. It's a violation
of DRY: you have typed the string "people" in two places, and your
program breaks if (a) the filename changes or (b) the way "people.txt"
is transformed into "People" changes. Maybe you _want_ that breakage
(maybe you want the program to fail if someone tries to run it on
"other-people.txt" or on "places.txt"). Or maybe not: it's another
tradeoff. (To be fair, the article doesn't claim that the version with
People hard-coded can read places.txt.)
3. Is it really a good idea to encourage people to eval("[#{line}]") ???