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.

Sign up for GitHub

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

Jump to bottom

Karis/Melissa - Edges - Adagrams #18

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Karis/Melissa - Edges - Adagrams #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
94704ea
created letter_bag array for wave 1
kimj42 Aug 14, 2018
7ea02f9
Added method to check if uses_available_letters? method to find if le…
kimj42 Aug 14, 2018
151a1f7
added false to uses_available_letters? method
kimj42 Aug 14, 2018
cba720b
outputs true or false depending on adagram
kimj42 Aug 15, 2018
b3cd03b
returns false if contains repeated letters more than the drawn letters
kimj42 Aug 15, 2018
6b477b5
Created a hash of the letters, and split them and put them in a new a…
melicious-dish Aug 16, 2018
c140a5e
added if conditional to add 8 points for a longer word
melicious-dish Aug 16, 2018
db19097
got the best score into a hash
kimj42 Aug 16, 2018
28184f8
in case of tied score, prefers the word with fewer letters
kimj42 Aug 16, 2018
6c8e98b
in case of tied score, prefers the word with fewer letters regardless…
kimj42 Aug 17, 2018
f4afbd0
passes all tests including preferring word with 10 letters in case of…
kimj42 Aug 17, 2018
493877d
added comments for work
melicious-dish Aug 17, 2018
ea1ea73
Create adagrams.rb
kimj42 Aug 17, 2018
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
117 changes: 117 additions & 0 deletions adagrams.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# chooses 10 letters randomly from the letter bag
def draw_letters()
letter_bag = ["A", "A", "A", "A", "A", "A", "A", "A", "A", "B", "B", "C", "C", "D", "D", "D", "D", "E", "E", "E", "E", "E", "E", "E", "E", "E", "E", "E", "E", "F", "F", "G", "G", "G", "H", "H", "I", "I", "I", "I", "I", "I", "I", "I", "I", "J", "K", "L", "L", "L", "L", "M", "M", "N", "N", "N", "N", "N", "N", "O", "O", "O", "O", "O", "O", "O", "O", "P", "P", "Q", "R", "R", "R", "R", "R", "R", "S", "S", "S", "S", "T", "T", "T", "T", "T", "T", "U", "U", "U", "U", "V", "V", "W", "W", "X", "Y", "Y", "Z"]

# randomly selects the letters
Copy link

Choose a reason for hiding this comment

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

This data structure makes sense for storing the pool of letters. However, laid out on one line like this it's not easy to see the distribution of letters. Something like this might be a bit easier to read:

letters = [
  "A", "A", "A", "A", "A", "A", "A", "A", "A",
  "B", "B",
  "C", "C",
  ...
]

Or, if you were feeling fancy:

letter_distribution = {
  A: 9,
  B: 2,
  C: 2,
  ...
}
letters = []
letter_distribution.each do |letter, count|
  count.times do
    letters << letter
  end
end

ten_letters = letter_bag.sample(10)

return ten_letters
end

# checks to see if adagram that was created
# includes drawn letters only
def uses_available_letters?(input, letters_in_hand)

# takes word and puts it into an array
adagram = input.split(//)
Copy link

Choose a reason for hiding this comment

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

There are two problems with this implementation:

  1. You will never return true for a word with a repeated letter. For example,

    hand = ["E", "F", "R", "A", "E", "L", "V", "A", "D", "K"]
uses_available_letters?("ADA", hand)
# => false

    The reason why is that Ruby's Array#delete method will remove all matching elements from an array, not just the first match.

  2. This method is destructive! Because you call delete on the input, 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
# => ["E", "F", "R", "A", "E", "L", "V", "A"]

    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.

I will acknowledge that none of the tests checked for either of these behaviors, but that doesn't change the fact that they're bugs.


# checks each letter against drawn letters
# to see if they are included
adagram.each do |letter|
return false unless letters_in_hand.include?(letter)

# check for repeated letters more than in drawn letters
if letters_in_hand.include?(letter)
letters_in_hand.delete(letter)
end
end

return true
end

def score_word(word)
score_chart =
{
"A" => 1, "E" => 1, "I" => 1, "O" => 1, "U" => 1, "L" => 1, "N" => 1, "R" => 1, "S" => 1, "T" => 1, "D" => 2, "G" => 2, "B" => 3, "C" => 3, "M" => 3, "P" => 3, "F" => 4, "H" => 4, "V" => 4, "W" => 4, "Y" => 4, "K" => 5, "J" => 8, "X" => 8, "Q" => 10, "Z" => 10
}
Copy link

Choose a reason for hiding this comment

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

Again, this data structure feels like a good fit here, but I think it would be more readable if you were to split it across multiple lines.


#Find the sum of letters passed into the method
total_score = 0

#split up the word and put into an array to iterate
#make all letters caps
letters = word.upcase.split('')
letters.each do |character|
total_score += score_chart[character]
end

# adds 8 points to total score if given word
# is more than 7 letters
if letters.length >= 7
total_score = total_score + 8
end
return total_score
end

# finds highest score from given words
def highest_score_from(words)

# calculates the score of each word
score_of_each_word = words.map do |word|
score_word(word)
end

# stores the word and its score in a hash
words_with_scores = Hash.new
words.zip(score_of_each_word).each do |word, score|
words_with_scores[word] = score
end

# stores word with highest score in a hash of its own
highest_score_words = []

# creates new hash that contains the given word
# and its score only if the score has the
# highest score
words_with_scores.each do |word, score|
if score == words_with_scores.values.max
best_word = Hash.new
Copy link

Choose a reason for hiding this comment

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

This works! However!

On line 77, you're calling values.max inside an each loop. That means you're doing the work of finding the max value (O(n)) once for each word (n times), for a total time complexity of O(n^2). We can improve this to O(n) by moving the max outside the loop:

max_score = words_with_scores.values.max
words_with_scores.each do |word, score|
    if score == max_score
# ...

best_word[:word] = word
best_word[:score] = score
highest_score_words << best_word
end
end

# if 2 or more words have the same high score
# all those words will be compared to the first
# best scoring word
if highest_score_words.length >= 2
best_scoring_word = highest_score_words[0][:word]
number_of_letters_of_best_word = best_scoring_word.length

# looks at the first word and counts the number of letters
# use that first word to compare to the rest of the words
highest_score_words.each do |letters|
word = letters[:word]
letter_count = word.length

# considers ties, and chooses the word with the fewest lertters. If the tie contains words with 10 letters, that will be the best scoring word, regardless of order.
if letter_count < number_of_letters_of_best_word
number_of_letters_of_best_word = letter_count
best_scoring_word = word
elsif
letter_count == 10
number_of_letters_of_best_word = letter_count
best_scoring_word = word
final_answer = {:word => best_scoring_word, :score => score_word(best_scoring_word)}
return final_answer
end
end

final_answer = {:word => best_scoring_word, :score => score_word(best_scoring_word)}
return final_answer
end

# displays the hash that has the best word and score. returns the first word in case if a tie with the same length
return highest_score_words.first
end
117 changes: 117 additions & 0 deletions lib/adagrams.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# chooses 10 letters randomly from the letter bag
def draw_letters()
Copy link

Choose a reason for hiding this comment

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

You have included the same file twice, as adagrams.rb and lib/adagrams.rb. Probably only the lib one should be included here.

letter_bag = ["A", "A", "A", "A", "A", "A", "A", "A", "A", "B", "B", "C", "C", "D", "D", "D", "D", "E", "E", "E", "E", "E", "E", "E", "E", "E", "E", "E", "E", "F", "F", "G", "G", "G", "H", "H", "I", "I", "I", "I", "I", "I", "I", "I", "I", "J", "K", "L", "L", "L", "L", "M", "M", "N", "N", "N", "N", "N", "N", "O", "O", "O", "O", "O", "O", "O", "O", "P", "P", "Q", "R", "R", "R", "R", "R", "R", "S", "S", "S", "S", "T", "T", "T", "T", "T", "T", "U", "U", "U", "U", "V", "V", "W", "W", "X", "Y", "Y", "Z"]

# randomly selects the letters
ten_letters = letter_bag.sample(10)

return ten_letters
end

# checks to see if adagram that was created
# includes drawn letters only
def uses_available_letters?(input, letters_in_hand)

# takes word and puts it into an array
adagram = input.split(//)

# checks each letter against drawn letters
# to see if they are included
adagram.each do |letter|
return false unless letters_in_hand.include?(letter)

# check for repeated letters more than in drawn letters
if letters_in_hand.include?(letter)
letters_in_hand.delete(letter)
end
end

return true
end

def score_word(word)
score_chart =
{
"A" => 1, "E" => 1, "I" => 1, "O" => 1, "U" => 1, "L" => 1, "N" => 1, "R" => 1, "S" => 1, "T" => 1, "D" => 2, "G" => 2, "B" => 3, "C" => 3, "M" => 3, "P" => 3, "F" => 4, "H" => 4, "V" => 4, "W" => 4, "Y" => 4, "K" => 5, "J" => 8, "X" => 8, "Q" => 10, "Z" => 10
}

#Find the sum of letters passed into the method
total_score = 0

#split up the word and put into an array to iterate
#make all letters caps
letters = word.upcase.split('')
letters.each do |character|
total_score += score_chart[character]
end

# adds 8 points to total score if given word
# is more than 7 letters
if letters.length >= 7
total_score = total_score + 8
end
return total_score
end

# finds highest score from given words
def highest_score_from(words)

# calculates the score of each word
score_of_each_word = words.map do |word|
score_word(word)
end

# stores the word and its score in a hash
words_with_scores = Hash.new
words.zip(score_of_each_word).each do |word, score|
words_with_scores[word] = score
end

# stores word with highest score in a hash of its own
highest_score_words = []

# creates new hash that contains the given word
# and its score only if the score has the
# highest score
words_with_scores.each do |word, score|
if score == words_with_scores.values.max
best_word = Hash.new
best_word[:word] = word
best_word[:score] = score
highest_score_words << best_word
end
end

# if 2 or more words have the same high score
# all those words will be compared to the first
# best scoring word
if highest_score_words.length >= 2
best_scoring_word = highest_score_words[0][:word]
number_of_letters_of_best_word = best_scoring_word.length

# looks at the first word and counts the number of letters
# use that first word to compare to the rest of the words
highest_score_words.each do |letters|
word = letters[:word]
letter_count = word.length

# considers ties, and chooses the word with the fewest lertters. If the tie contains words with 10 letters, that will be the best scoring word, regardless of order.
if letter_count < number_of_letters_of_best_word
number_of_letters_of_best_word = letter_count
best_scoring_word = word
elsif
letter_count == 10
number_of_letters_of_best_word = letter_count
best_scoring_word = word
final_answer = {:word => best_scoring_word, :score => score_word(best_scoring_word)}
return final_answer
end
end

final_answer = {:word => best_scoring_word, :score => score_word(best_scoring_word)}
return final_answer
end

# displays the hash that has the best word and score. returns the first word in case if a tie with the same length
return highest_score_words.first
end