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

Tor and Angela - Octos - Scrabble #3

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

Conversation

torshimizu
Copy link

Scrabble

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the advantages of using git when collaboratively working on one code base? can both make edits, and if working on same bit of code, git will know if you need to resolve conflicts. Can have one repo to collect all commits and changes from all collaborators involved.
Why do you think we chose to make the score method in the Scoring class a class method instead of an instance method? There is no need to create an instance of score - no state is needed to keep track of.
Describe an example of a test you wrote for a nominal case. We had a test to confirm that the TileBag.draw_tiles method would successfully remove the drawn tiles from the instance of TileBag.
Describe an example of a test you wrote for an edge case. We added a test to confirm that the first word was returned in the case of a tie between two seven letter words.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? Yes, we used them to calculate a player's high score, and it helped keep the code concise from the confusion of nested loops.
What was one method you and your pair used to debug code? We used a few methods depending on the problem, but relied on binding.pry to check our logic, or the contents of variables.
Is there a specific piece of code you'd like feedback on? We're wondering if there is a simpler way to handle Scrabble::Scoring.highest_score_from
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? 1. We both are very open to each other's solutions, and open minded about choosing the most practical solution. 2. For this specific pairing, ping-pong worked well as a strategy for pair programming, but we both realize that this doesn't work for all pairings.

torshimizu and others added 29 commits February 20, 2018 14:21
@kariabancroft
Copy link

Scrabble

What We're Looking For

Feature Feedback
General
Answered comprehension questions Yes
Both Teammates contributed to the codebase Both folks did commit - but I'd like to see more from Angela in pairs in the future
Regular Commits with meaningful commit messages Good frequency
Scoring class
score method
Uses appropriate data structure to store the letter score Yes - though your implementation is not the most efficient. You'll have to iterate through the letters collection for every single letter in the word. Also, putting the values into an array and then summing them up is less efficient than just creating the sum of the values right off the bat.
Appropriately handles edge cases Yes - you have an opportunity here for "short circuiting" your solution. Checking if the word is greater than the max should happen before the word score is established because you're wasting iterations calculating a value that will never be used.
highest_score_from method Nice just of the Hash to keep track of words and their scores.
Appropriately handles edge cases Yes - nice job handling ties and max word length.
Test implementations for highest_score_from Yes - looks good, though I did have one comment about a tie for max # letters
Player class
Uses an array to keep track of words played Yes
Uses existing code to create highest_scoring_word and highest_word_score Yes!
Returns true or false from won? method Yes - nice use of the ternary
Tests edge cases on the play(word) Looks good - in the future I'd definitely recommend using the before block in your tests to DRY things up.
TileBag class
Uses a data structure to keep the set of default tiles in the bag Yes - I like the way you use the overall hash to store the initial values and then expand that out in the initialize method.
draw_tiles method uses the parameter to draw a set of random tiles Yes
tiles_remaining method returns a count Yes
Tests draw_tiles edge cases Yes - looks good

Overall you did a nice job with this assignment. You exercised the major learning goals including iterators, enumerables, testing and other fun stuff!


winner = all_highscores[0]
if all_highscores.any? { |word| word.length == MAX_WORD_LENGTH }
seven_letter_score = all_highscores.select { |word|

Choose a reason for hiding this comment

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

If you expect that this could only be one winner (by the [0] below) you could just use find instead of select

end

it 'if tied, prefer a word with 7 letters' do
word_array = ['qqzzqq', 'aaaaadb']

Choose a reason for hiding this comment

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

How would these be tied scores?

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