Skip to content

wrote a shakespeare analyzer #13

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.

Sign up for GitHub

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

Open
shreve wants to merge 4 commits into
base: master
Choose a base branch
from
Open

wrote a shakespeare analyzer #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d2662b
wrote a shakespeare analyzer
shreve Nov 27, 2013
94e4ffa
Added a missing test, and added an output file
shreve Nov 27, 2013
0e2b417
In the last commit I changed print_lines_per_speaker! to lines_per_sp…
shreve Nov 27, 2013
f7ef901
Refactor classes based on feedback from @r00k
shreve Jan 3, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 8 additions & 30 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
```
16 changes: 16 additions & 0 deletions app
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
40 changes: 40 additions & 0 deletions lib/play.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
require 'nokogiri'
require 'open-uri'
require_relative 'speech'

class Play
attr_accessor :document

def initialize(play_location)
Copy link
Contributor

@r00k r00k Dec 19, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about play_uri for a name here?

Copy link
Contributor

@r00k r00k Dec 19, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nevermind.

@play_location = play_location
end

def speeches
document.css('SPEECH').map do |speech|
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
34 changes: 34 additions & 0 deletions lib/shakespeare_analyzer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
require_relative 'play'

class ShakespeareAnalyzer
attr_accessor :play, :speeches, :speakers

def initialize(play_location)
@play = Play.new(play_location)
@speakers = Hash.new(0)
end

def lines_per_speaker
most_verbose_speakers.map { |name, line_count| "#{line_count}\t#{name}\n" }.join
end

private
def most_verbose_speakers
Copy link
Contributor

@r00k r00k Dec 19, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good method name!

analyze_speeches
@speakers.sort_by { |name, line_count| -line_count }
end

def get_speeches
@speeches ||= @play.speeches
end

def analyze_speeches
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] += speech.lines.count
end
end
end
end
11 changes: 11 additions & 0 deletions lib/speech.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Speech
attr_accessor :speakers, :lines

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
41 changes: 41 additions & 0 deletions shakespeare_analyzer_out.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions test/example_play.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<PLAY>
<TITLE>A Tale of Two Rubyists: The Chunkiest Bacon</TITLE>
<ACT>
<TITLE>ACT I</TITLE>
<SCENE>
<TITLE>SCENE I. An Introduction</TITLE>
<SPEECH>
<SPEAKER>FOX 1</SPEAKER>
<LINE>Lorem ipsum dolor sit amet...</LINE>
</SPEECH>
<SPEECH>
<SPEAKER>FOX 2</SPEAKER>
<LINE>This is boring, I'm out.</LINE>
Copy link
Contributor

@r00k r00k Dec 19, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lulz.

</SPEECH>
<SPEECH>
<SPEAKER>FOX 1</SPEAKER>
<SPEAKER>FOX 2</SPEAKER>
<LINE>CHUNKY BACON!</LINE>
</SPEECH>
</SCENE>
</ACT>
</PLAY>
27 changes: 27 additions & 0 deletions test/play_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

@r00k r00k Dec 19, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you didn't do work in Plays initializer, you could check that it initialized correctly without calling open on this url.

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
16 changes: 16 additions & 0 deletions test/shakespeare_analyzer_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
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

def test_lines_per_speaker_outputs_data_correctly
assert_equal "2\tFox 1\n2\tFox 2\n", @analyzer.lines_per_speaker,
"lines_per_speaker output didn't match expected"
Copy link
Contributor

@r00k r00k Dec 19, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this failure message tells you more than what the default message would.

Copy link
Author

@shreve shreve Jan 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really think of a better message that accurately describes what went wrong.

Does this suggest the test is poorly written (i.e. a code smell)?

Copy link
Contributor

@r00k r00k Jan 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily. I was just pointing out that I'd probably remove the message here, since I don't think it will give you additional information beyond rspec's default failure message.

Copy link
Author

@shreve shreve Jan 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok. I've got you now.

end
end
23 changes: 23 additions & 0 deletions test/speech_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions test/test_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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