On 18 May 2008, at 18:28, Jens Carroll wrote:
I was not aware that "@xml_import.user.country.should
equal(@country)" is already
a trainwreck. I think I have even longer ones - refactoring might be
the magic
word for my code now.
Well I guess it's more the wrong sort of leaves on the line :) It's
not so much the length of the line that's the problem, that's just a
code smell. The real issue is that it makes more sense for the user
(domain model) to update itself off the XML, rather than have an XML
parser (utility class) go poking around inside the user.
Having had a second look at it, I'd be inclined to say that
XmlImport#parse_xml should really be a class method of User (something
like User.import_all_from_xml). The only bit of this method that
isn't domain logic is the line
@doc = Hpricot.XML(open(file))
which should probably be in the controller, or wherever this code gets
called. You don't want the domain logic depending on filesystem
technicalities and more than the utility code fiddling with the model.
Ashley
--
http://www.patchspace.co.uk/
http://aviewfromafar.net/
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
