Skip to content
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

Jack Chester Solutions #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Binary file added spec/.DS_Store
Binary file not shown.
43 changes: 43 additions & 0 deletions spec/wordcounter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require 'rspec'
Copy link
Member

Choose a reason for hiding this comment

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

love the fact that you wrote tests for this. 👍

require_relative '../wordcounter'

describe '#countWords' do
let(:counter) { Counter.new }

context 'when string is unique' do
it 'returns proper array' do
expect(counter.countWords("maverick goose iceman")).to eq(
[["maverick", 1],
["goose", 1],
["iceman", 1]])
end
end

context 'when string is nonunique' do
it 'returns proper array' do
expect(counter.countWords("maverick maverick goose maverick iceman goose")).to eq(
[["maverick", 3],
["goose", 2],
["iceman", 1]])
end
end

context 'when string is nonunique, puntuated, with capitalization' do
Copy link
Member

Choose a reason for hiding this comment

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

you can actually nests contexts.

it 'returns proper array' do
expect(counter.countWords("GOOSE! Maverick Goose goose, Iceman (icEman)")).to eq(
Copy link
Member

Choose a reason for hiding this comment

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

[["goose", 3],
["iceman", 2],
["maverick", 1]])
end
end
end

describe '#printCount' do
let(:counter) { Counter.new }

context 'when reading in file' do
it 'does not raise an error' do
expect{counter.printCount("./speech.txt")}.not_to raise_error
end
end
end
25 changes: 25 additions & 0 deletions wordcounter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
class Counter

def countWords(text)
text = text.downcase.gsub(/[^a-z' ]/, '') #make lowercase and only keep allowed characters
Copy link
Member

Choose a reason for hiding this comment

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

so the indentation is might funky here, eh?

raw_words = text.split(" ")
count = {}
Copy link
Member

Choose a reason for hiding this comment

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

anytime you initialize a new object just to build it up later is a good time to think about using .map or .reduce.

in this case .reduce or .each_with_object is the way you want to go here. These would also simplify your code greatly.

Copy link
Author

Choose a reason for hiding this comment

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

👍 bookmarked the Ruby enumerable documentation


raw_words.each do |word|
if !count.key? word
count[word] = 1
else
count[word] += 1
end
end

count.sort_by{ |k, v| -v}
end

def printCount(path)
Copy link
Member

Choose a reason for hiding this comment

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

love the encapsulation. nitpick: keep in mind that lower snakecase is preferred by rubyists

Copy link
Author

Choose a reason for hiding this comment

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

👍 Thanks for the feedback!

text = File.open(path) { |line| line.read } #concatenates every line into one string
countWords(text).each do |word, count|
puts "#{word} - #{count}"
end
end
end