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
Open
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
55 changes: 55 additions & 0 deletions markov.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
require 'pp'
class Markov
attr_accessor :source_text, :hash

def initialize(file)
@source_text = File.read(file)
parse_source
build_frequency_hash
end

def output(length = 12)
@some_word = @hash.to_a.sample(1)[0][0]
@output = @some_word
@capitalize = false
@length = 0
length.times do
core_loop
end
core_loop until @output[-1] == '.' || @length > length + 100
@output[0] = @output[0].upcase
@output = @output + '.' unless @output[-1] == '.'
@output.gsub!(/[)(]/, '')
@output
end

def core_loop
@length += 1
@capitalize = true if @some_word[-1] == '.'
@capitalize = true if @some_word[-2] == '.'
@some_word = @hash[@some_word].sample(1)[0] || '.'

if @capitalize
@some_word.capitalize!
@capitalize = false
end
@output += ' ' unless @output[-1] == ' '
@output += @some_word
end

def parse_source
@words = @source_text.split(/[ \n]/)
end

def build_frequency_hash
@hash = {}
@words.each_cons(2) do |two_words|
next unless two_words.length == 2
if !@hash[two_words[0]]
@hash[two_words[0]] = [two_words[1]]
else
@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. ;)

end