-
-
Notifications
You must be signed in to change notification settings - Fork 50
Shakespeare Analyzer Solution #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| --color | ||
| --format progress |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| require 'nokogiri' | ||
| require 'open-uri' | ||
| require_relative 'line_counter' | ||
|
|
||
| include Nokogiri | ||
|
|
||
| class CallbackHandler < XML::SAX::Document | ||
| def initialize | ||
| @counter = LineCounter.new | ||
| @parsing_speaker = false | ||
| end | ||
|
|
||
| def results | ||
| @counter.count | ||
| end | ||
|
|
||
| def start_element(element, attributes) | ||
| @counter.count_line if element == 'LINE' | ||
| @counter.new_speech if element == 'SPEECH' | ||
| @counter.new_scene if element == 'SCENE' | ||
| @parsing_speaker = (element == 'SPEAKER') | ||
| end | ||
|
|
||
| def end_element(element) | ||
| @parsing_speaker = false if element == 'SPEAKER' | ||
| end | ||
|
|
||
| def characters string | ||
| @counter.add_speaker string if @parsing_speaker | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| require 'set' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that you split up the responsibilities so that line counting was separate from parsing. |
||
|
|
||
| class LineCounter | ||
| def initialize | ||
| @speaker = [] | ||
| @scene_speakers = Set.new | ||
| @count = Hash.new(0) | ||
| end | ||
|
|
||
| def count | ||
| @count | ||
| end | ||
|
|
||
| def add_speaker(name) | ||
| @speaker.push name | ||
| @scene_speakers.add name unless name == 'ALL' | ||
| end | ||
|
|
||
| def count_line | ||
| names = (@speaker.include?("ALL")) ? @scene_speakers : @speaker | ||
| increment_names(names) | ||
| raise ArgumentError, 'Unable to count lines without a speaker.' if @speaker.empty? | ||
| end | ||
|
|
||
| def increment_names(names) | ||
| names.each do |name| | ||
| @count[name] += 1 | ||
| end | ||
| end | ||
|
|
||
| def new_speech | ||
| @speaker = [] | ||
| end | ||
|
|
||
| def new_scene | ||
| @scene_speakers = Set.new | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| require_relative 'callback_handler' | ||
|
|
||
| handler = CallbackHandler.new | ||
| XML::SAX::Parser.new(handler).parse_io(open("http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be nice to extract the URL into a constant.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of duck typing, letting the parser grab the file! |
||
| results = handler.results.sort_by {|k,v| v}.reverse | ||
| results.each {|k,v| puts " #{v} #{k.capitalize}" } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| 729 Macbeth | ||
| 267 Lady macbeth | ||
| 215 Malcolm | ||
| 183 Macduff | ||
| 136 Ross | ||
| 115 Banquo | ||
| 83 First witch | ||
| 76 Lennox | ||
| 70 Duncan | ||
| 48 Porter | ||
| 48 Third witch | ||
| 48 Second witch | ||
| 46 Hecate | ||
| 45 Doctor | ||
| 41 Lady macduff | ||
| 35 Sergeant | ||
| 31 Siward | ||
| 30 First murderer | ||
| 23 Gentlewoman | ||
| 23 Messenger | ||
| 21 Angus | ||
| 21 Lord | ||
| 20 Son | ||
| 15 Second murderer | ||
| 12 Donalbain | ||
| 12 Menteith | ||
| 11 Old man | ||
| 11 Caithness | ||
| 8 Third apparition | ||
| 8 Second apparition | ||
| 8 Third murderer | ||
| 7 Young siward | ||
| 6 First apparition | ||
| 5 Servant | ||
| 5 Seyton | ||
| 3 Lords | ||
| 2 Fleance | ||
| 2 Both murderers | ||
| 1 Soldiers | ||
| 1 Attendant |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| <?xml version="1.0"?> | ||
| <!DOCTYPE PLAY SYSTEM "play.dtd"> | ||
| <PLAY> | ||
| <ACT> | ||
| <SCENE> | ||
| <TITLE>SCENE I. A desert place.</TITLE> | ||
| <STAGEDIR>Thunder and lightning. Enter three Witches</STAGEDIR> | ||
| <SPEECH> | ||
| <SPEAKER>First Witch</SPEAKER> | ||
| <LINE>When shall we three meet again</LINE> | ||
| <LINE>In thunder, lightning, or in rain?</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>Second Witch</SPEAKER> | ||
| <LINE>When the hurlyburly's done,</LINE> | ||
| <LINE>When the battle's lost and won.</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>Third Witch</SPEAKER> | ||
| <LINE>That will be ere the set of sun.</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>First Witch</SPEAKER> | ||
| <LINE>Where the place?</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>Second Witch</SPEAKER> | ||
| <LINE>Upon the heath.</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>Third Witch</SPEAKER> | ||
| <LINE>There to meet with Macbeth.</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>First Witch</SPEAKER> | ||
| <LINE>I come, Graymalkin!</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>Second Witch</SPEAKER> | ||
| <LINE>Paddock calls.</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>Third Witch</SPEAKER> | ||
| <LINE>Anon.</LINE> | ||
| </SPEECH> | ||
| <SPEECH> | ||
| <SPEAKER>ALL</SPEAKER> | ||
| <LINE>Fair is foul, and foul is fair:</LINE> | ||
| <LINE>Hover through the fog and filthy air.</LINE> | ||
| </SPEECH> | ||
| <STAGEDIR>Exeunt</STAGEDIR> | ||
| </SCENE> | ||
| </ACT> | ||
| </PLAY> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| require 'callback_handler' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice clean tests here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good use of fixtures. |
||
|
|
||
| describe CallbackHandler do | ||
| before (:each) do | ||
| @handler = CallbackHandler.new | ||
| @parser = XML::SAX::Parser.new(@handler) | ||
| end | ||
|
|
||
| it "correctly parses an empty file" do | ||
| @parser.parse_file("spec/empty_file.xml") | ||
| @handler.results.keys.should be_empty | ||
| end | ||
|
|
||
| it "correctly parses a file with only one speaker" do | ||
| @parser.parse_file("spec/one_speaker.xml") | ||
| @handler.results.keys.should_not be_empty | ||
| @handler.results["MACBETH"].should eq(1) | ||
| end | ||
|
|
||
| it "correctly parses a file with multiple speakers" do | ||
| @parser.parse_file("spec/multiple_speakers.xml") | ||
| @handler.results.keys.should_not be_empty | ||
|
|
||
| @handler.results["LENNOX"].should eq(1) | ||
| @handler.results["MACBETH"].should eq(2) | ||
| @handler.results["MACDUFF"].should eq(6) | ||
| end | ||
|
|
||
| it "correctly parses a file with a speaker of all" do | ||
| @parser.parse_file("spec/all_speaker.xml") | ||
| @handler.results.keys.should_not be_empty | ||
|
|
||
| @handler.results["First Witch"].should eq(6) | ||
| @handler.results["Second Witch"].should eq(6) | ||
| @handler.results["Third Witch"].should eq(5) | ||
| @handler.results["ALL"].should eq(0) | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| require 'line_counter' | ||
|
|
||
| describe LineCounter, "#count" do | ||
| before (:each) do | ||
| @counter = LineCounter.new | ||
| end | ||
|
|
||
| it "returns an empty list when no lines are counted" do | ||
| @counter.count.should be_empty | ||
| end | ||
|
|
||
| it "increments the current speaker when a speaker is added" do | ||
| @counter.add_speaker "Macbeth" | ||
|
|
||
| @counter.count_line | ||
| @counter.count["Macbeth"].should eq(1) | ||
|
|
||
| @counter.count_line | ||
| @counter.count["Macbeth"].should eq(2) | ||
| end | ||
|
|
||
| it "throws exception when a line is counted without a speaker" do | ||
| expect {@counter.count_line}.to raise_error(ArgumentError) | ||
| end | ||
|
|
||
| it "allows two people to speak at the same time" do | ||
| @counter.add_speaker "Macbeth" | ||
| @counter.add_speaker "Banquo" | ||
|
|
||
| @counter.count_line | ||
| @counter.count["Macbeth"].should eq(1) | ||
| @counter.count["Banquo"].should eq(1) | ||
|
|
||
| @counter.count_line | ||
| @counter.count["Macbeth"].should eq(2) | ||
| @counter.count["Banquo"].should eq(2) | ||
| end | ||
|
|
||
| it "does not count lines from speakers of previous speeches" do | ||
| @counter.add_speaker "Macbeth" | ||
|
|
||
| @counter.new_speech | ||
|
|
||
| @counter.add_speaker "Duncan" | ||
| @counter.count_line | ||
|
|
||
| @counter.count["Duncan"].should eq(1) | ||
| @counter.count.key?("Macbeth").should be_false | ||
| end | ||
|
|
||
| it "increments all speakers in the scene when the speaker is all" do | ||
| @counter.add_speaker "Macbeth" | ||
| @counter.count_line | ||
|
|
||
| @counter.new_speech | ||
| @counter.add_speaker "Banquo" | ||
| @counter.count_line | ||
| @counter.count_line | ||
|
|
||
| @counter.new_speech | ||
| @counter.add_speaker "ALL" | ||
| @counter.count_line | ||
|
|
||
| @counter.count["Macbeth"].should eq(2) | ||
| @counter.count["Banquo"].should eq(3) | ||
| @counter.count["ALL"].should eq(0) | ||
| end | ||
|
|
||
| it "increments only the speakers in the scene when the speaker is all" do | ||
| @counter.add_speaker "Macbeth" | ||
| @counter.count_line | ||
|
|
||
| @counter.new_scene | ||
| @counter.new_speech | ||
| @counter.add_speaker "Banquo" | ||
| @counter.count_line | ||
|
|
||
| @counter.new_speech | ||
| @counter.add_speaker "Duncan" | ||
| @counter.count_line | ||
| @counter.count_line | ||
|
|
||
| @counter.new_speech | ||
| @counter.add_speaker "ALL" | ||
| @counter.count_line | ||
|
|
||
| @counter.count["Macbeth"].should eq(1) | ||
| @counter.count["Banquo"].should eq(2) | ||
| @counter.count["Duncan"].should eq(3) | ||
| @counter.count["ALL"].should eq(0) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| <?xml version="1.0"?> | ||
| <!DOCTYPE PLAY SYSTEM "play.dtd"> | ||
| <PLAY> | ||
| <SPEECH> | ||
| <SPEAKER>MACDUFF</SPEAKER> | ||
| <LINE>O horror, horror, horror! Tongue nor heart</LINE> | ||
| <LINE>Cannot conceive nor name thee!</LINE> | ||
| </SPEECH> | ||
|
|
||
| <SPEECH> | ||
| <SPEAKER>MACBETH</SPEAKER> | ||
| <SPEAKER>LENNOX</SPEAKER> | ||
| <LINE>What's the matter.</LINE> | ||
| </SPEECH> | ||
|
|
||
| <SPEECH> | ||
| <SPEAKER>MACDUFF</SPEAKER> | ||
| <LINE>Confusion now hath made his masterpiece!</LINE> | ||
| <LINE>Most sacrilegious murder hath broke ope</LINE> | ||
| <LINE>The Lord's anointed temple, and stole thence</LINE> | ||
| <LINE>The life o' the building!</LINE> | ||
| </SPEECH> | ||
|
|
||
| <SPEECH> | ||
| <SPEAKER>MACBETH</SPEAKER> | ||
| <LINE>What is 't you say? the life?</LINE> | ||
| </SPEECH> | ||
| </PLAY> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| <?xml version="1.0"?> | ||
| <!DOCTYPE PLAY SYSTEM "play.dtd"> | ||
| <PLAY> | ||
| <SPEECH> | ||
| <SPEAKER>MACBETH</SPEAKER> | ||
| <LINE>Methought I heard a voice cry 'Sleep no more!</LINE> | ||
| </SPEECH> | ||
| </PLAY> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # This file was generated by the `rspec --init` command. Conventionally, all | ||
| # specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. | ||
| # Require this file using `require "spec_helper"` to ensure that it is only | ||
| # loaded once. | ||
| # | ||
| # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration | ||
| RSpec.configure do |config| | ||
| config.treat_symbols_as_metadata_keys_with_true_values = true | ||
| config.run_all_when_everything_filtered = true | ||
| config.filter_run :focus | ||
|
|
||
| # Run specs in random order to surface order dependencies. If you find an | ||
| # order dependency and want to debug it, you can fix the order by providing | ||
| # the seed, which is printed after each run. | ||
| # --seed 1234 | ||
| config.order = 'random' | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.