Skip to content

Conversation

@sahanamurthy
Copy link

Scrabble

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
How do you feel you and your partner did in sharing responsibilities? We think that we shared the responsibilities well -- we alternated between writing the program and writing the specs. Cynthia did a little more navigating and Sahana did a little more driving.
Why do you think we chose to make the score method in the Scoring class a class method instead of an instance method? Because we didn't need to make an instance of Score. We needed the Scoring class to act like a calculator.
Describe an example of a test you wrote for a nominal case. In Player, we tested normal and appropriate dictionary words that would be played in Scrabble.
Describe an example of a test you wrote for an edge case. In our edge case test, we tested for a one letter word and the special seven letter bonus words.
What was your final test coverage percentage? 100% (but we're not bragging)
Is there a specific piece of code you'd like feedback on? We would like feedback on how we handled dependencies. Did we isolate the dependency on calling the Scoring class inside the Player class. Also, did we appropriately use the single responsibility principle throughout our Classes and methods?

Sahana Murthy and others added 30 commits February 27, 2017 15:49
…t draft of Player class initialize program.
cynthiacd and others added 20 commits March 1, 2017 13:54
@droberts-sea
Copy link

Scrabble

What We're Looking For

Feature Feedback
General
Answered comprehension questions yes
Both Teammates contributed to the codebase yes
Regular Commits with meaningful commit messages yes
Scoring class
score method yes
Uses appropriate data structure to store the letter score yes - excellent work, making the right decision here makes the entire rest of this wave easier
Appropriately handles edge cases Would like to see more defensive programming - things like checking input validity first, etc.
Tests score edge cases (one-letter word, 7 letter word) Edge cases are handled, but missing logic for failure cases (empty string, not a string, invalid characters, etc)
highest_scoring_word method logic is mostly there - see inline comment
Appropriately handles edge cases yes, good work checking input
Tests highest_scoring_word edge cases (ties, no words, no array) yes
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
Tests edge cases on the play(word) method for words that cannot be scored yes
TileBag class
Uses a data structure to keep the set of default tiles in the bag yes
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 for different parameter inputs some - the only test is for non-integers, but for example negative numbers are not tested

Great work overall! Code is clean and readable, and git hygiene looks good. Test coverage is mostly spot on. There were a few failure cases I would like to see covered explicitly - most of these are called out above. In general, keep up the good work.

# The & calls to_proc on the object
# Returns a proc object that expects a parameter and calls a method
# parameter is the tie_words and the method is length
return tie_words.min_by(&:length)
Copy link

@droberts-sea droberts-sea Mar 7, 2017

Choose a reason for hiding this comment

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

Good work doing the research on procs. However, I'm not sure this implementation will work in every case - I think line 17 needs to be outside the each loop. As a test case, try ['zzzzzz', 'aaaaaah']

Choose a reason for hiding this comment

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

Also good work abstracting this as its own method!


describe "Scoring#highest_score_from" do

before do

Choose a reason for hiding this comment

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

Nice use of before here.

attr_reader :tile_bag

def initialize
@tile_bag = [ "A", "A", "A", "A", "A", "A", "A", "A", "A", "B", "B", "C",

Choose a reason for hiding this comment

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

While impressive-looking, a big literal array like this is hard to manage. Say your boss comes in with the news that Scrabble should now include 9 "E"s - how many do you have now? You'll have to count, which is error prone.

A better way of storing this information might be something like you did in the Scoring class, a hash where each letter maps to its number of tiles. You'd then want to iterate over it to generate the tile bag itself.

end

def draw_tiles(num)
raise ArgumentError unless num.class == Integer

Choose a reason for hiding this comment

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

What about negative numbers?

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