Skip to content

Conversation

@meaganelizabeth
Copy link

This is close. The parser still counts dashes and it couldn't hurt to write tests for the TextParser class.

Copy link
Member

@mikegee mikegee left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me.


def count_words
parsed_text.each do |word|
if word_counts.key?(word)
Copy link
Member

Choose a reason for hiding this comment

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

If you instantiate the word_counts hash with a default value of 0, you wouldn't need to check if the word has already been seen.

http://ruby-doc.org/core-2.2.2/Hash.html#method-c-new

end

def sorted_counts
word_counts.sort_by {|_key, value| -value}
Copy link
Member

Choose a reason for hiding this comment

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

I bet there are people that wouldn't think this is readable, but I love the simplicity of it.

It would be a slight improvement to give _key and value more intention-revealing names. _word and count, I suppose.

Copy link

Choose a reason for hiding this comment

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

+1. Variables should have meaningful names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants