-
-
Notifications
You must be signed in to change notification settings - Fork 50
My attempt #7
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?
My attempt #7
Changes from all commits
91c73ab
041f392
a916afe
32e8c9c
4c1a14b
9d04705
523500d
4baccde
b4facfe
f98f919
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,8 @@ | ||
| #!/usr/bin/env ruby | ||
|
|
||
| require_relative '../lib/shakespeare_analyzer' | ||
| require_relative '../lib/downloader' | ||
|
|
||
| downloader = Downloader.new file: "http://www.ibiblio.org/xml/examples/shakespeare/macbeth.xml" | ||
| shakes = ShakespeareAnalyzer.new xml_provider: downloader | ||
| shakes.print_lines | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| require 'open-uri' | ||
|
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 job isolating this responsibility into one class! |
||
|
|
||
| class Downloader | ||
|
|
||
| def initialize options = {} | ||
| @file_to_download = options.fetch :file | ||
| end | ||
|
|
||
| def get_xml | ||
| complete_file = "" | ||
| open(@file_to_download) do |f| | ||
| f.each_line { |line| complete_file << line.rstrip } | ||
| end | ||
| complete_file | ||
| end | ||
|
|
||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| require 'rexml/document' | ||
|
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. It would be interesting to know why you chose this library over Nokogiri. Are there any trade-offs?
Author
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 don't really know enough about Nokogiri or REXML to know the tradeoffs involved, but since this was a learning exercise I wanted to try to stick to the standard library. That's all. |
||
|
|
||
| class ShakespeareAnalyzer | ||
|
|
||
| def initialize options = {} | ||
| @xml_provider = options[:xml_provider] | ||
|
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 wouldn't use an options hash for just one argument. |
||
| @file = REXML::Document.new xml | ||
| end | ||
|
|
||
| def print_lines | ||
| in_descending_order(read_lines).each do |key, value| | ||
| puts "#{key}: #{value} lines" | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def xml | ||
| @xml_provider.get_xml | ||
| end | ||
|
|
||
| def read_lines | ||
| characters = Hash.new 0 | ||
|
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 call on using Hash.new with the default value. |
||
| @file.elements.each("/PLAY//LINE") do |line| | ||
| characters[current_speaker(line)] += 1 | ||
| end | ||
| characters | ||
| end | ||
|
|
||
| def current_speaker line | ||
| line.parent.elements[1].text | ||
| end | ||
|
|
||
| def in_descending_order characters | ||
|
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. Nicely done. |
||
| characters.sort_by { |key, value| -value } | ||
|
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 idea to use |
||
| end | ||
|
|
||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| require 'downloader' | ||
|
|
||
| describe Downloader do | ||
|
|
||
| it "returns data from a file or url" do | ||
| downloader = Downloader.new(file: "./spec/test.xml") | ||
| test_file = downloader.get_xml | ||
| expect(test_file).to include "Macbeth" | ||
| end | ||
|
|
||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| require 'ostruct' | ||
| require 'shakespeare_analyzer' | ||
|
|
||
| describe ShakespeareAnalyzer do | ||
|
|
||
| testfile = OpenStruct.new get_xml: %Q| | ||
| <PLAY> | ||
| <ACT> | ||
| <SCENE> | ||
| <SPEECH> | ||
| <SPEAKER>Benjamin Sisko</SPEAKER> | ||
| <LINE>Don't you understand? They're REAL!</LINE> | ||
| <LINE>I CREATED THEM!</LINE> | ||
| <LINE>You cannot destroy an IDEA!</LINE> | ||
| </SPEECH> | ||
| </SCENE> | ||
| </ACT> | ||
| </PLAY>| | ||
|
|
||
| it "can use a xml retriever object to retrieve a file" do | ||
| double_xmler = double("double xmler") | ||
| expect(double_xmler).to receive(:get_xml) | ||
| test_analyzer = ShakespeareAnalyzer.new xml_provider: double_xmler | ||
| end | ||
|
|
||
| it "prints the results" do | ||
| test_analyzer = ShakespeareAnalyzer.new xml_provider: testfile | ||
| expect(STDOUT).to receive(:puts).with("Benjamin Sisko: 3 lines") | ||
| test_analyzer.print_lines | ||
| end | ||
| 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 like
xml_provideras a name!