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

Lindsay + Katricia - Edges - Adagrams #24

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
136 changes: 136 additions & 0 deletions adagrams.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
require 'pry'

Choose a reason for hiding this comment

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

You've included this file twice, once as adagrams.rb, and once as lib/adagrams.rb.

#create data structure
all_letters = []

letters = {"A" => 9, "B" => 2, "C" => 2, "D"=> 4, "E" => 12, "F"=> 2, "G"=>3, "H"=>2, "I" => 9, "J"=>1, "K"=> 1, "L"=>4, "M"=> 2, "N"=> 6, "O"=>8, "P" => 2, "Q" => 1, "R"=>6, "S"=>4, "T"=>6, "U"=>4, "V"=>2, "W"=>2, "X"=>1, "Y"=>2, "Z"=>1}


# Fill pool of letters array with letters
letters.each do |letter, freq|
count = 0
until count == freq
all_letters << letter
count += 1
end
end


# Define method for creating user hand
def draw_letters(array)
user_letters = array.sample(10)
return user_letters
end

DRAWN_LETTERS = draw_letters(all_letters)


# Transform input word string into array, all in capital letters
def make_word_array(input)
input = input.upcase.split("")
return input
end


#create method to see if word uses letters that are actually available
def uses_available_letters? (input, drawn_letters)
is_valid = "default"
word_letters = make_word_array(input)
word_letters.each do |letter|
if drawn_letters.include?letter
is_valid = true
puts "letter valid " + "#{is_valid}"
letter_index = drawn_letters.index(letter)
drawn_letters.delete_at(letter_index)
else
is_valid = false
puts "letter not valid " + "#{is_valid}"
return is_valid
end
end
return is_valid
end


puts "drawn letters: #{DRAWN_LETTERS}"


#turn all_letters array into a hash that represents the score of each letter
LETTER_SCORES = {}

# Make hash for letter to score
all_letters.uniq.each do |letter|

case letter
when "A","E", "I", "O", "U", "L", "N", "R", "S", "T"
score = 1
when "D", "G"
score = 2
when "B", "C", "M", "P"
score = 3
when "F", "H", "V", "W", "Y"
score = 4
when "K"
score = 5
when "J", "X"
score = 8
when "Q", "Z"
score = 10
else
score = 0
end

LETTER_SCORES[letter] = score
end

puts "Letter point values: #{LETTER_SCORES}"

#create method for scoring word
def score_word(word)
word_letters = make_word_array(word)
score = 0
word_letters.each do |letter|
score += LETTER_SCORES[letter]
end
score += 8 if word.length >= 7 && word.length <= 10
return score
end


#create a method to return a hash with the highest scoring words
def highest_score_from(words)
all_words_scores = []

# Calculate scores of all words
words.each do |word|
each_word_score = {}
each_word_score[:word] = word
each_word_score[:score] = score_word(word)
all_words_scores << each_word_score
end

word_rank = all_words_scores.each.sort_by {|hash| hash[:score]}.reverse

puts "word rank: #{word_rank}"

best_word = word_rank[0]

word_rank.each_with_index do |hash, index|

if word_rank[index][:score] > best_word[:score]
best_word = word_rank[index]

# Tie Handling
elsif word_rank[index][:score] == best_word[:score]
if best_word[:word].length != 10
if word_rank[index][:word].length == 10
best_word = word_rank[index]
elsif word_rank[index][:word].length < best_word[:word].length
best_word = word_rank[index]
end
end
end
index += 1
end
return best_word
end
1 change: 1 addition & 0 deletions lib/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
adagramstest.rb
136 changes: 136 additions & 0 deletions lib/adagrams.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
require 'pry'

#create data structure
all_letters = []

letters = {"A" => 9, "B" => 2, "C" => 2, "D"=> 4, "E" => 12, "F"=> 2, "G"=>3, "H"=>2, "I" => 9, "J"=>1, "K"=> 1, "L"=>4, "M"=> 2, "N"=> 6, "O"=>8, "P" => 2, "Q" => 1, "R"=>6, "S"=>4, "T"=>6, "U"=>4, "V"=>2, "W"=>2, "X"=>1, "Y"=>2, "Z"=>1}


# Fill pool of letters array with letters
letters.each do |letter, freq|

Choose a reason for hiding this comment

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

I like the way you've chosen to store the letter frequencies in a hash and then dynamically turn them into an array. I would say that this is much more readable than a giant hard-coded array with 9 As, 2 Bs, 2 Cs, etc.

count = 0
until count == freq
all_letters << letter
count += 1
end
end


# Define method for creating user hand
def draw_letters(array)
user_letters = array.sample(10)
return user_letters
end

DRAWN_LETTERS = draw_letters(all_letters)

Choose a reason for hiding this comment

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

I think there was some confusion about what was expected here. The idea is that draw_letters should be called by the driver code (wave-X-game.rb) or by the specs in order to create a hand of letters. Your code should not ever need to call draw_letters.



# Transform input word string into array, all in capital letters
def make_word_array(input)
input = input.upcase.split("")
return input
end


#create method to see if word uses letters that are actually available
def uses_available_letters? (input, drawn_letters)
is_valid = "default"
word_letters = make_word_array(input)
word_letters.each do |letter|

Choose a reason for hiding this comment

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

This method is destructive! Because you call delete_at on the drawn_letters directly, the hand will be reduced by the letters in the input. For example:

hand = draw_letters
puts hand
# => ["T", "T", "E", "F", "R", "A", "E", "L", "V", "A"]
uses_available_letters?('tte', hand)
puts hand
# => ["F", "R", "A", "E", "L", "V", "A"]

While none of the tests we provided check for destroying the hand, this is bad because it's unexpected behavior. Nothing about "check whether this word is made up of letters in this hand" suggests that the hand will be changed in the process, but that's exactly what happens.

The way to address this problem would be to make a copy of the hand before removing letters from it.

if drawn_letters.include?letter
is_valid = true
puts "letter valid " + "#{is_valid}"
letter_index = drawn_letters.index(letter)

Choose a reason for hiding this comment

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

Please remove debugging output before submitting projects.

drawn_letters.delete_at(letter_index)
else
is_valid = false
puts "letter not valid " + "#{is_valid}"
return is_valid
end
end
return is_valid
end


puts "drawn letters: #{DRAWN_LETTERS}"


#turn all_letters array into a hash that represents the score of each letter
LETTER_SCORES = {}

# Make hash for letter to score
all_letters.uniq.each do |letter|

case letter
when "A","E", "I", "O", "U", "L", "N", "R", "S", "T"

Choose a reason for hiding this comment

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

The idea of generating this data structure dynamically with a loop and a case statement is an interesting one. However, I think I would argue that the resulting code is a little less readable than just hardcoding it:

LETTER_SCORES = {
  "A" => 1,
  "B" => 3,
  # ...
}

score = 1
when "D", "G"
score = 2
when "B", "C", "M", "P"
score = 3
when "F", "H", "V", "W", "Y"
score = 4
when "K"
score = 5
when "J", "X"
score = 8
when "Q", "Z"
score = 10
else
score = 0
end

LETTER_SCORES[letter] = score
end

puts "Letter point values: #{LETTER_SCORES}"

#create method for scoring word
def score_word(word)
word_letters = make_word_array(word)
score = 0
word_letters.each do |letter|
score += LETTER_SCORES[letter]
end
score += 8 if word.length >= 7 && word.length <= 10
return score
end


#create a method to return a hash with the highest scoring words
def highest_score_from(words)
all_words_scores = []

Choose a reason for hiding this comment

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

This implementation is close, but the tie-breaking logic isn't quite right. The problem is that you don't respect the order in which the words were given.

The reason this happens is the sorting you do on line 112. This will re-order the words by their scores, so that the input order is now gone.

As it turns out, you don't need to sort the words at all to find the max. If we change line 112 to word_rank = all_words_scores, the rest of your logic works perfectly and all the (uncommented) tests pass.

# Calculate scores of all words
words.each do |word|
each_word_score = {}
each_word_score[:word] = word
each_word_score[:score] = score_word(word)
all_words_scores << each_word_score
end

word_rank = all_words_scores.each.sort_by {|hash| hash[:score]}.reverse

puts "word rank: #{word_rank}"

best_word = word_rank[0]

word_rank.each_with_index do |hash, index|

if word_rank[index][:score] > best_word[:score]
best_word = word_rank[index]

# Tie Handling
elsif word_rank[index][:score] == best_word[:score]
if best_word[:word].length != 10
if word_rank[index][:word].length == 10
best_word = word_rank[index]
elsif word_rank[index][:word].length < best_word[:word].length

Choose a reason for hiding this comment

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

Good work getting this tricky tie-breaking logic figured out.

best_word = word_rank[index]
end
end
end
index += 1
end
return best_word
end
Loading