Skip to content

Conversation

njch5
Copy link

@njch5 njch5 commented Aug 15, 2019

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? Method signature, parameters, arguments.
What are the advantages of using git when collaboratively working on one code base? You can save and update changes and have multiple people working on the same project.
What kind of relationship did you and your pair have with the unit tests? We used rake to troubleshoot our methods.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? .map and .select
What was one method you and your pair used to debug code? We read the errors and also used Pry.
What are two discussion points that you and your pair discussed when giving/receiving feedback from each other that you would be willing to share? We learned a lot together and how to communicate with each other.

@beccaelenzil
Copy link

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions yes -- next time be a bit more thorough in your responses so that you demonstrate a strong reflection on process
Small commits with meaningful commit messages yes
Code Requirements
draw_letters method
Uses appropriate data structure to store the letter distribution yes
All tests for draw_letters pass yes
uses_available_letters? method
All tests for uses_available_letters? pass yes
score_word method
Uses appropriate data structure to store the letter scores yes
All tests for score_word pass yes
highest_score_from method
Appropriately handles edge cases for tie-breaking logic no
All tests for highest_score_from pass no
Overall Good work on this project. Your code is clear and readable. You make good use of a hash for creating the hand of letters. I see that you did not reach a resolution on the single test failure on the highest_score method. My biggest recommendation here is to try to move away from nested conditional control structures to simplify your logic and use comments to outline the rules of the tiebreaker method.

require "pry"

def draw_letters
alphabet_soup = {

Choose a reason for hiding this comment

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

Consider making this hash a global variable using all caps that is defined outside the method.


def uses_available_letters?(input, letters_in_hand)
dup_array = letters_in_hand.dup

Choose a reason for hiding this comment

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

When you are returning the value returned from a block of code, explicitly assign that value to a variable, and then return that variable.

input.upcase.split(//).all? do |letter|
  included = dup_array.include?(letter)
  dup_array.delete(letter)
end
return included

score = 0

word_array = word.upcase.split(//)
word_array.each do |letter|

Choose a reason for hiding this comment

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

While this case statement works, it means that the information about which letter has which score is locked into this piece of code, and can't easily be used elsewhere. For example, if you wanted to display the value of each letter in a hand, you would need to repeat this work.

An alternative approach would be to store the letter scores in a hash, something like this:

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

Then to get the score for a letter, you can say LETTER_SCORES[letter].

winners = new_hash.select { |word, score| score == max_score }
winners_array = winners.keys

winners_array.each do |word|

Choose a reason for hiding this comment

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

The logic here is a bit complicated and you still have not passed that one test:

  test_0006_in case of tied score, prefers most the word with 10 letters regardless of order FAIL (0.00s)
        Expected: "AAAAAAAAAA"
          Actual: "BBBBBB"

It seems that if instead of nesting if/else control structures a chained conditional would simplify your logic. Here is such a chained conditional for a tiebreaker between 2 words.

def tiebreaker(word1, word2)
    if word1.length == 10
      return word1
    elsif word2.length == 10
      return word2
    elsif word1.length < word2.length
      return word1
    elsif word1.length > word2.length
      return word2
    else
      word1
    end
  end

winners = new_hash.select { |word, score| score == max_score }
winners_array = winners.keys

winners_array.each do |word|

Choose a reason for hiding this comment

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

Consider using comments to outline the logic of the tiebreakers rules

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