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

Dan and Sage seattle.rb code retreat #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ragesoss
Copy link

@ragesoss ragesoss commented Apr 5, 2017

No description provided.

@hash[two_words[0]] << two_words[1]
end
end
end
Copy link
Collaborator

@phiggins phiggins Apr 8, 2017

Choose a reason for hiding this comment

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

Hey, here's a small bit of code review.

  • each_cons will only ever generate arrays of the requested size, so the length check is unnecessary.

  • Ruby's destructuring can be used to get rid of some of these array accesses and make the code a little easier to read.

  • Hash.new takes a block that is run when an element is looked up and not found, that can be used to set a default value.

Putting it all together, you could rewrite this method to look something like:

def build_frequency_hash
  @hash = Hash.new {|h,k| h[k] = [] }
  @words.each_cons(2) do |word, follower|
    @hash[word] << follower
  end
end

If you wanted to make this a stateless function, you could get even crazier:

def build_frequency_hash(words)
  words.each_cons(2).with_object(Hash.new {|h,k| h[k] = [] }) do |(word, follower), hash|
    hash[word] << follower
  end
end

Your code is fine, now I'm just golfing. ;)

@phiggins
Copy link
Collaborator

phiggins commented Apr 8, 2017

This is great, thanks for sharing! 👍

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.

2 participants