From 4d2662bb35675accefa7b411fe5ed603f000f391 Mon Sep 17 00:00:00 2001 From: Jacob Evan Shreve Date: Wed, 27 Nov 2013 17:19:22 -0500 Subject: [PATCH 1/4] wrote a shakespeare analyzer --- README.md | 38 +++++++------------------------ app.rb | 4 ++++ lib/play.rb | 24 +++++++++++++++++++ lib/shakespeare_analyzer.rb | 36 +++++++++++++++++++++++++++++ lib/speech.rb | 12 ++++++++++ test/example_play.xml | 22 ++++++++++++++++++ test/play_test.rb | 27 ++++++++++++++++++++++ test/shakespeare_analyzer_test.rb | 13 +++++++++++ test/speech_test.rb | 23 +++++++++++++++++++ test/test_helper.rb | 11 +++++++++ 10 files changed, 180 insertions(+), 30 deletions(-) create mode 100644 app.rb create mode 100644 lib/play.rb create mode 100644 lib/shakespeare_analyzer.rb create mode 100644 lib/speech.rb create mode 100644 test/example_play.xml create mode 100644 test/play_test.rb create mode 100644 test/shakespeare_analyzer_test.rb create mode 100644 test/speech_test.rb create mode 100644 test/test_helper.rb diff --git a/README.md b/README.md index 11daf64..abf363b 100644 --- a/README.md +++ b/README.md @@ -1,33 +1,11 @@ -***An exercise for Prime subscribers. Visit http://learn.thoughtbot.com/prime to learn more.*** +# Shakespeare Play Analyzer +(like wc for theater) -### Difficulty level: intermediate. -## Your Task +#### How Can I Use This Wonderful Library? -As a Shakespeare buff, statistics junkie, and unix lover, Ben finds himself wanting a command-line tool for analyzing Macbeth. - -Write a command-line program that prints the number of lines spoken by each character in the play. - -Sample usage/output (using made-up numbers): - - $ ruby macbeth_analyzer.rb - 543 Macbeth - 345 Banquo - 220 Duncan - (etc.) - -You can find an XML-encoded version of Macbeth here: http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml. Your program should download and parse this file at runtime. - -Your solution must be tested, preferably via TDD. - -## Working/Submitting - -1. To work on this exercise, fork the repo and begin implementing your solution. -2. When you are done, copy the output of your program into a file in this repository. -3. Create a pull request so your code can be reviewed. -4. Perform a code review on at least one other person's solution. Your comments should follow our code review guidelines: https://github.com/thoughtbot/guides/tree/master/code-review. Most important: be friendly. Make suggestions, not demands. -5. Improve your solution based on the comments you've received and approaches you've learned from reviewing others' attempts. - -## Bounty - -While knowledge and skill improvement are their own rewards, the author with the best solution (as judged by thoughtbot) will receive a cool thoughtbot t-shirt. +``` +git clone https://github.com/shreve/shakespeare_analyzer.git +cd shakespeare_analyzer +ruby app.rb +``` \ No newline at end of file diff --git a/app.rb b/app.rb new file mode 100644 index 0000000..9de430e --- /dev/null +++ b/app.rb @@ -0,0 +1,4 @@ +require_relative 'lib/shakespeare_analyzer.rb' + +@analyzer = ShakespeareAnalyzer.new "http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml" +@analyzer.print_lines_per_speaker! diff --git a/lib/play.rb b/lib/play.rb new file mode 100644 index 0000000..404843b --- /dev/null +++ b/lib/play.rb @@ -0,0 +1,24 @@ +require 'nokogiri' +require 'open-uri' +require_relative 'speech' + +class Play + attr_accessor :document + + def initialize(play_location) + unless play_location.is_a? File + if play_location.match(/^http/) + play_location = open(play_location) + else + play_location = File.open(play_location, 'r') + end + end + @document = Nokogiri::XML(play_location) + end + + def speeches + document.css('SPEECH').map do |speech| + Speech.new speech + end + end +end diff --git a/lib/shakespeare_analyzer.rb b/lib/shakespeare_analyzer.rb new file mode 100644 index 0000000..b97b7d6 --- /dev/null +++ b/lib/shakespeare_analyzer.rb @@ -0,0 +1,36 @@ +require_relative 'play' + +class ShakespeareAnalyzer + attr_accessor :play, :speeches, :speakers + + def initialize(play_location) + @play = Play.new(play_location) + analyze_speeches + end + + def print_lines_per_speaker! + most_verbose_speakers.each do |name, line_count| + puts "#{line_count}\t#{name}" + end + end + + private + def most_verbose_speakers + @speakers.sort_by { |name, line_count| -line_count } + end + + def get_speeches + @speeches ||= @play.speeches + end + + def analyze_speeches + @speakers ||= {} + get_speeches.each do |speech| + # nested iterators :/ + speech.speakers.each do |speaker| + # new value is previous value or zero plus the number of lines in the speech + @speakers[speaker] = ( @speakers[speaker] || 0 ) + speech.lines.count + end + end + end +end diff --git a/lib/speech.rb b/lib/speech.rb new file mode 100644 index 0000000..e83b6ad --- /dev/null +++ b/lib/speech.rb @@ -0,0 +1,12 @@ +class Speech + attr_accessor :speakers, :lines + + # speech_xml is a Nokogiri xml object + def initialize(speech_xml) + @speakers = speech_xml.css('SPEAKER').map do |speaker| + # I'd do map(&:content), but I hate CAPSLOCKED SPEAKER NAMES + speaker.content.downcase.split.map(&:capitalize).join(' ') + end + @lines = speech_xml.css('LINE').map(&:content) + end +end diff --git a/test/example_play.xml b/test/example_play.xml new file mode 100644 index 0000000..28b7cd9 --- /dev/null +++ b/test/example_play.xml @@ -0,0 +1,22 @@ + + A Tale of Two Rubyists: The Chunkiest Bacon + + ACT I + + SCENE I. An Introduction + + FOX 1 + Lorem ipsum dolor sit amet... + + + FOX 2 + This is boring, I'm out. + + + FOX 1 + FOX 2 + CHUNKY BACON! + + + + diff --git a/test/play_test.rb b/test/play_test.rb new file mode 100644 index 0000000..39856c9 --- /dev/null +++ b/test/play_test.rb @@ -0,0 +1,27 @@ +class TestPlay < MiniTest::Unit::TestCase + def setup + @play = Play.new(example_play) + end + + def test_play_initializes_with_file_path + play = Play.new(example_play_path) + assert play.document, "play was unable to initialize when given a file path as a string" + end + + def test_play_initializes_with_file_instance + assert @play.document, "play was unable to initialize when given a File object" + end + +# This passed, but my internet sucks, so I don't +# want to fetch the play every time I run my tests + def test_play_initializes_with_url + play = Play.new("http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml") + assert play.document, "play was unable to initialize when given a url" + end + + def test_speeches_initializes_an_array_of_speech_objects + speeches = @play.speeches + assert speeches.is_a? Array + assert speeches.first.is_a? Speech + end +end diff --git a/test/shakespeare_analyzer_test.rb b/test/shakespeare_analyzer_test.rb new file mode 100644 index 0000000..723bd3a --- /dev/null +++ b/test/shakespeare_analyzer_test.rb @@ -0,0 +1,13 @@ +require_relative '../lib/shakespeare_analyzer.rb' +require 'minitest/autorun' +require_relative 'test_helper' +require_relative 'play_test' +require_relative 'speech_test' + +class TestShakespeareAnalyzer < MiniTest::Unit::TestCase + def setup + @analyzer = ShakespeareAnalyzer.new example_play + end + + +end diff --git a/test/speech_test.rb b/test/speech_test.rb new file mode 100644 index 0000000..10733b8 --- /dev/null +++ b/test/speech_test.rb @@ -0,0 +1,23 @@ +require 'nokogiri' + +class TestSpeech < MiniTest::Unit::TestCase + def setup + @speech = Speech.new(speech_xml.first) + end + + def test_speech_initializes_with_attributes + assert @speech.speakers && @speech.lines + end + + def test_speakers_are_properly_capitalized + assert_equal "Fox 1", @speech.speakers.first + end + + def test_lines_are_properly_added + assert_equal 1, @speech.lines.count + end + + def speech_xml + nk_example_play.css('SPEECH') + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb new file mode 100644 index 0000000..e2041e5 --- /dev/null +++ b/test/test_helper.rb @@ -0,0 +1,11 @@ +def example_play_path + File.join(File.dirname(__FILE__), 'example_play.xml') +end + +def example_play + File.open(example_play_path, 'r') +end + +def nk_example_play + Nokogiri::XML(example_play) +end From 94e4ffaf1170ed4c2a67a5d8463ae776ef4f50f3 Mon Sep 17 00:00:00 2001 From: Jacob Evan Shreve Date: Wed, 27 Nov 2013 17:41:47 -0500 Subject: [PATCH 2/4] Added a missing test, and added an output file --- app.rb | 2 +- lib/shakespeare_analyzer.rb | 7 ++++-- shakespeare_analyzer_out.txt | 41 +++++++++++++++++++++++++++++++ test/shakespeare_analyzer_test.rb | 5 +++- 4 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 shakespeare_analyzer_out.txt diff --git a/app.rb b/app.rb index 9de430e..bec0c07 100644 --- a/app.rb +++ b/app.rb @@ -1,4 +1,4 @@ require_relative 'lib/shakespeare_analyzer.rb' @analyzer = ShakespeareAnalyzer.new "http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml" -@analyzer.print_lines_per_speaker! +@analyzer.lines_per_speaker diff --git a/lib/shakespeare_analyzer.rb b/lib/shakespeare_analyzer.rb index b97b7d6..5fc9b3b 100644 --- a/lib/shakespeare_analyzer.rb +++ b/lib/shakespeare_analyzer.rb @@ -8,10 +8,13 @@ def initialize(play_location) analyze_speeches end - def print_lines_per_speaker! + def lines_per_speaker(write=true) + output = "" most_verbose_speakers.each do |name, line_count| - puts "#{line_count}\t#{name}" + output << "#{line_count}\t#{name}\n" end + print output if write + output end private diff --git a/shakespeare_analyzer_out.txt b/shakespeare_analyzer_out.txt new file mode 100644 index 0000000..218fd46 --- /dev/null +++ b/shakespeare_analyzer_out.txt @@ -0,0 +1,41 @@ +719 Macbeth +265 Lady Macbeth +212 Malcolm +180 Macduff +135 Ross +113 Banquo +74 Lennox +70 Duncan +62 First Witch +46 Porter +45 Doctor +41 Lady Macduff +39 Hecate +35 Sergeant +30 Siward +30 First Murderer +27 Third Witch +27 Second Witch +24 All +23 Gentlewoman +23 Messenger +21 Angus +21 Lord +20 Son +15 Second Murderer +12 Menteith +11 Old Man +11 Caithness +10 Donalbain +8 Third Murderer +7 Young Siward +5 Third Apparition +5 Seyton +5 Servant +4 Second Apparition +3 Lords +2 First Apparition +2 Both Murderers +2 Fleance +1 Attendant +1 Soldiers diff --git a/test/shakespeare_analyzer_test.rb b/test/shakespeare_analyzer_test.rb index 723bd3a..5cb2ea7 100644 --- a/test/shakespeare_analyzer_test.rb +++ b/test/shakespeare_analyzer_test.rb @@ -9,5 +9,8 @@ def setup @analyzer = ShakespeareAnalyzer.new example_play end - + def test_lines_per_speaker_outputs_data_correctly + assert_equal "2\tFox 1\n2\tFox 2\n", @analyzer.lines_per_speaker(false), + "lines_per_speaker output didn't match expected" + end end From 0e2b4173b8cacd1aa71d0e6cffe01859bd6559af Mon Sep 17 00:00:00 2001 From: Jacob Evan Shreve Date: Wed, 27 Nov 2013 17:46:52 -0500 Subject: [PATCH 3/4] In the last commit I changed print_lines_per_speaker! to lines_per_speaker, but was still making it print. I updated this so it makes more sense. --- app.rb | 2 +- lib/shakespeare_analyzer.rb | 3 +-- test/shakespeare_analyzer_test.rb | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app.rb b/app.rb index bec0c07..f12fd61 100644 --- a/app.rb +++ b/app.rb @@ -1,4 +1,4 @@ require_relative 'lib/shakespeare_analyzer.rb' @analyzer = ShakespeareAnalyzer.new "http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml" -@analyzer.lines_per_speaker +print @analyzer.lines_per_speaker diff --git a/lib/shakespeare_analyzer.rb b/lib/shakespeare_analyzer.rb index 5fc9b3b..8ef5c49 100644 --- a/lib/shakespeare_analyzer.rb +++ b/lib/shakespeare_analyzer.rb @@ -8,12 +8,11 @@ def initialize(play_location) analyze_speeches end - def lines_per_speaker(write=true) + def lines_per_speaker output = "" most_verbose_speakers.each do |name, line_count| output << "#{line_count}\t#{name}\n" end - print output if write output end diff --git a/test/shakespeare_analyzer_test.rb b/test/shakespeare_analyzer_test.rb index 5cb2ea7..507afb9 100644 --- a/test/shakespeare_analyzer_test.rb +++ b/test/shakespeare_analyzer_test.rb @@ -10,7 +10,7 @@ def setup end def test_lines_per_speaker_outputs_data_correctly - assert_equal "2\tFox 1\n2\tFox 2\n", @analyzer.lines_per_speaker(false), + assert_equal "2\tFox 1\n2\tFox 2\n", @analyzer.lines_per_speaker, "lines_per_speaker output didn't match expected" end end From f7ef901c425c77b74716515a64c83e328a368d96 Mon Sep 17 00:00:00 2001 From: Jacob Evan Shreve Date: Thu, 2 Jan 2014 21:01:47 -0500 Subject: [PATCH 4/4] Refactor classes based on feedback from @r00k --- app | 16 ++++++++++++++++ app.rb | 4 ---- lib/play.rb | 32 ++++++++++++++++++++++++-------- lib/shakespeare_analyzer.rb | 12 ++++-------- lib/speech.rb | 1 - 5 files changed, 44 insertions(+), 21 deletions(-) create mode 100755 app delete mode 100644 app.rb diff --git a/app b/app new file mode 100755 index 0000000..8019590 --- /dev/null +++ b/app @@ -0,0 +1,16 @@ +#!/usr/bin/env ruby + +require_relative 'lib/shakespeare_analyzer.rb' + +MACBETH_URL = "http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml" +HAMLET_URL = "http://www.ibiblio.org/xml/examples/shakespeare/hamlet.xml" + +macbeth_analyzer = ShakespeareAnalyzer.new MACBETH_URL +hamlet_analyzer = ShakespeareAnalyzer.new HAMLET_URL + +puts "Lines Speaker" +puts "Macbeth" +print macbeth_analyzer.lines_per_speaker + +puts "Hamlet" +print hamlet_analyzer.lines_per_speaker diff --git a/app.rb b/app.rb deleted file mode 100644 index f12fd61..0000000 --- a/app.rb +++ /dev/null @@ -1,4 +0,0 @@ -require_relative 'lib/shakespeare_analyzer.rb' - -@analyzer = ShakespeareAnalyzer.new "http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml" -print @analyzer.lines_per_speaker diff --git a/lib/play.rb b/lib/play.rb index 404843b..00cd7d1 100644 --- a/lib/play.rb +++ b/lib/play.rb @@ -6,14 +6,7 @@ class Play attr_accessor :document def initialize(play_location) - unless play_location.is_a? File - if play_location.match(/^http/) - play_location = open(play_location) - else - play_location = File.open(play_location, 'r') - end - end - @document = Nokogiri::XML(play_location) + @play_location = play_location end def speeches @@ -21,4 +14,27 @@ def speeches Speech.new speech end end + + def document + @document ||= Nokogiri::XML(play_data) + end + + private + def play_data + if location_is_a_file? + @play_location + elsif location_is_a_url? + open(@play_location) + else + File.open(@play_location, 'r') + end + end + + def location_is_a_file? + @play_location.is_a? File + end + + def location_is_a_url? + @play_location.match(/^http/) + end end diff --git a/lib/shakespeare_analyzer.rb b/lib/shakespeare_analyzer.rb index 8ef5c49..1b9e6bc 100644 --- a/lib/shakespeare_analyzer.rb +++ b/lib/shakespeare_analyzer.rb @@ -5,19 +5,16 @@ class ShakespeareAnalyzer def initialize(play_location) @play = Play.new(play_location) - analyze_speeches + @speakers = Hash.new(0) end def lines_per_speaker - output = "" - most_verbose_speakers.each do |name, line_count| - output << "#{line_count}\t#{name}\n" - end - output + most_verbose_speakers.map { |name, line_count| "#{line_count}\t#{name}\n" }.join end private def most_verbose_speakers + analyze_speeches @speakers.sort_by { |name, line_count| -line_count } end @@ -26,12 +23,11 @@ def get_speeches end def analyze_speeches - @speakers ||= {} get_speeches.each do |speech| # nested iterators :/ speech.speakers.each do |speaker| # new value is previous value or zero plus the number of lines in the speech - @speakers[speaker] = ( @speakers[speaker] || 0 ) + speech.lines.count + @speakers[speaker] += speech.lines.count end end end diff --git a/lib/speech.rb b/lib/speech.rb index e83b6ad..8926890 100644 --- a/lib/speech.rb +++ b/lib/speech.rb @@ -1,7 +1,6 @@ class Speech attr_accessor :speakers, :lines - # speech_xml is a Nokogiri xml object def initialize(speech_xml) @speakers = speech_xml.css('SPEAKER').map do |speaker| # I'd do map(&:content), but I hate CAPSLOCKED SPEAKER NAMES