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

Sockets - Chantal & Tatiana #24

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

Conversation

tatsqui
Copy link

@tatsqui tatsqui commented Feb 23, 2019

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? Method signature, parameters(if any) block, return value.
What are the advantages of using git when collaboratively working on one code base? Easily view and edit file, without the other person (not having to email copies to each other). It tracks all changes and keeps a log of them (when comitted).
What kind of relationship did you and your pair have with the unit tests? A tumultous one. We learned how to read unit testing a lot better and actually understand how it works and apply that to what kind of code we need to write. Helpful for debugging as well. The relationship with the tests did get better.
Does your code use any methods from the Enumerable mixed in? If so, where and why was it helpful? we used a map method to return the word/score for finding out what word won.
What was one method you and your pair used to debug code? Pry, printing to the screen, unit tests were helpful.
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 discussed out learning styles and execution styles with our coding techniques and recognized it would be great to go over that from the start and come up with a plan of action to use each other's strengths before tackling individual problems in the project.

Copy link

@droberts-sea droberts-sea left a comment

Choose a reason for hiding this comment

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

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions yes
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, but see inline
score_word method
Uses appropriate data structure to store the letter scores see inline
All tests for score_word pass yes
highest_score_from method
Appropriately handles edge cases for tie-breaking logic yes
All tests for highest_score_from pass yes
Overall

Good job overall! I've left a few inline comments, and you need to get your indentation figured out, but in general I'm happy with what I see here. It is clear to me that the learning goals for this assignment were met. Keep up the hard work!

end


draw_letters

Choose a reason for hiding this comment

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

You don't need to call draw_letters here. Your job is to define the method, the tests and the wave_X_game.rb will handle calling it.

{ word: element, score: score_word(element) }
end

find_best_word = []

Choose a reason for hiding this comment

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

Watch your indentation through here. Have you installed rubocop and rufo to automatically adjust your formatting?

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

Choose a reason for hiding this comment

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

This data would be a great thing to store in a constant!

if index.nil?
return false
else
letters_in_hand.delete_at(index)

Choose a reason for hiding this comment

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

When you call delete_at here, you modify the original hand of letters! While that makes the tests pass, it also introduces a bug:

hand = ['A', 'B', 'C']
word = 'ABC'
puts uses_available_letters?(word, hand)
# => true
puts uses_available_letters?(word, hand)
# => false

How might you address this problem?

case letter
when "A", 'E', 'I', 'O', 'U', 'L', 'R', 'N', 'S', 'T'
score += 1
when 'D', 'G'

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].

end
end
p find_best_word
winner = find_best_word.first

Choose a reason for hiding this comment

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

Please remove debugging output (like line 100) before submitting projects.

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.

3 participants