Shakespeare Analyzer Solution #4
Conversation
|
Really nice work, Brian! I am curious about your use of the more complex parsing with the CallBack Handler. I used Nokogiri as well, but just did the simpler "loop through the elements" approach. Is that because you're already familiar with that approach, or is there some additional benefit for this exercise? |
|
@twosevenzero Since all of them call for the same setup, definitely more DRY to have a before(:each) in there. Good catch! |
|
@JESii Thanks! I started with a SAX parser because a shakespeare play in XML format seemed like it would be "large" to read in to memory just to turn around and walk the tree again. With SAX parsing, it just reads the file once and calls those events so is generally faster and uses less memory than DOM. Turns out that the file is only 160K, so speed and memory probably isn't much of a factor. |
…scene instead of a speaker named "ALL".
|
@r00k I may be over-thinking the problem, but I noticed in the play there is a "speaker" of ALL. I updated my code/tests to attribute lines to all people previously in the scene, but the results seem like they could be a bit off. For example: in Act IV Scene I there is a line ("A deed without a name.") where the ALL is a response to Macbeth. In that case I think it's really more of an "All Witches" instead of "All people in this scene". Also, it seems like ALL could be affected by stage directions of a person leaving ("retires") like Hecate a couple lines above. |
|
@briantemple - you're absolutely right on the 'ALL' question; it would require parsing the elements, among others. |
|
@briantemple - / lib / line_counter.rb L22 |
|
@briantemple - Kinda cool how you structured your code so that pretty much all the "work" is done in the LineCounter class. Did that approach evolve out of your TDD or is that an approach you've used in the past? |
|
@JESii The way that the events are handled, the @speaker variable shouldn't be connected to any of the defined personas. So the only way that exception should occur is if, within the context of a speech, a line is spoken without a speaker being defined. This is something that should never happen, but unfortunately did when I was first writing the callback handler and wasn't sending values correctly. O:) Thanks -- I'm glad you liked my solution! :) Since I started with the SAX parser, I knew I would be getting events in a specific order and needed to handle the state of who was talking when a line occurred. Originally it only handled a single speaker, but morphed when I saw the multiple speakers and added tests for that. (I wish I had committed often to show the changes -- I got in to the fun of building and forgot. Oops!) I guess I started with the approach, and it naturally got fatter as more logic was required. As a bonus to the SAX approach, after I saw there was an 'ALL' speaker, it was really simple to add a hook to start a new scene and track all the people coming in to the scene. Same could go for stage directions adding or removing people before 'ALL' lines. I'm not sure how I would implement that equivalently in pure css/xpath line counts. I guess SAX fits well with sequential information like this. |
There was a problem hiding this comment.
I try to inject dependencies like these, rather than creating them inside my classes.
Sandy Metz's POODR has some good discussion of this, but basically it's better to not have such an explicit dependency in the class (your class HAS to use LineCounter), and to instead rely on duck-typing letting you pass in ANY object that responds to the right methods.
|
Solid effort! |
|
Thanks for the feedback, @r00k ! I really appreciate the book recommendation too - POODR is already ordered and on the way! :) |
|
Good call! I think you'll like it a lot. I did. On Fri, Sep 27, 2013 at 11:54 AM, Brian Temple [email protected]:
|
4 participants
This was fun -- thanks!