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

Scrabble Assignment #23

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

Scrabble Assignment #23

wants to merge 41 commits into from

Conversation

amanzan1
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? The advantages are that you can view your changes on git through git log and push changes to your partner's computer. Git is able to some extent merge both of you and your partner's changes.
Why do you think we chose to make the score method in the Scoring class a class method instead of an instance method? I think because it does not have a state to remember.
Describe an example of a test you wrote for a nominal case. One test we wrote was checking to see if tiles drawn were being removed from the tile_bag, so that actual number of tiles equaled the expected amount of tiles, which was a default number.
Describe an example of a test you wrote for an edge case. An example would be writing a test that would return nil if no words were passed. We created a word collection variable that was passed into the highest.score_from method and requiring that output to be nil.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? No.
What was one method you and your pair used to debug code? Running the rake command and using the puts method to print out what the program is doing.
Is there a specific piece of code you'd like feedback on? I can't think of anything at the moment.
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 thought the process went smoothly.

@CheezItMan
Copy link

Scrabble

What We're Looking For

Feature Feedback
General
Answered comprehension questions Check, You can't actually push changes to your partner's computer. Instead you push changes up to github and they can pull the changes down to their computer.
Both Teammates contributed to the codebase Both teammates made commits!
Regular Commits with meaningful commit messages Good number of commits, but focus more on what code you wrote in the commits and less on the waves completed. That would make it easier to rollback a commit if you need to later.
Scoring class
score method Check
Uses appropriate data structure to store the letter score Check, although the structure isn't the most efficient way to do it. See my inline notes.
Appropriately handles edge cases Check
highest_score_from method Check
Appropriately handles edge cases Small bug here, see my inline notes
Test implementations for highest_score_from Missing a test for tied words and both are 7-letters long. Your code returns the last word in case of a tie, if it's 7-letters long without considering there might be a high-scoring word that's seven letters long earlier.
Player class
Uses an array to keep track of words played Check
Uses existing code to create highest_scoring_word and highest_word_score Check
Returns true or false from won? method Check
Tests edge cases on the play(word) Pretty good, see my notes.
TileBag class
Uses a data structure to keep the set of default tiles in the bag Very good choice in using an Array
draw_tiles method uses the parameter to draw a set of random tiles Check, but see my note on drawing too many tiles.
tiles_remaining method returns a count Check
Tests draw_tiles edge cases You didn't test drawing more tiles than are in the bag.
Summary There is some good code here and some good testing. There are some serious bugs, notably in the highest_score_from, so look at that and look at how your tests missed that situation. You need to look at testing like a detective. Try to think about what could go wrong with your code. Read my notes and let me know any questions you have.

letters_array.each do |letter|
if ("a".."z").cover?(letter)
case
when letter == "d" || letter == "g"

Choose a reason for hiding this comment

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

This structure isn't the most efficient way to do it. Can you see a way to do this more compactly with a hash?

end

def self.highest_score_from(array_of_words)
array_of_words = array_of_words

Choose a reason for hiding this comment

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

This line doesn't actually do anything.

highest_score = value
highest_word = word

elsif (value == highest_score) && (word.length == 7)

Choose a reason for hiding this comment

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

You also need to check that highest_word isn't 7-letters long. You're missing a case.

end

it 'if tied, prefer a word with 7 letters' do
#Act

Choose a reason for hiding this comment

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

What about having 2 tied 7-letter words?

end
end

describe 'plays' do

Choose a reason for hiding this comment

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

You should also try to play words after the player has already won.


end

def draw_tiles(number)

Choose a reason for hiding this comment

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

What about when the user tries to draw more tiles than remain in the tile bag.

original_number_of_tiles = grab_bag.tiles_remaining
grab_bag.draw_tiles(6)

expected_number_of_tiles = original_number_of_tiles - 6

Choose a reason for hiding this comment

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

good test

end

end

Choose a reason for hiding this comment

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

You should also test drawing more tiles than remain in the bag.


end

xdescribe 'tiles_remaining' do

Choose a reason for hiding this comment

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

What is this test skipped?


number_of_tiles = grab_bag.tiles_remaining

grab_bag.remaining_tiles.length.must_equal number_of_tiles

Choose a reason for hiding this comment

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

better to hard-code the number of tiles.

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