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

Sara and Selam Amperes Scrabble #17

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

Conversation

SelamawitA
Copy link

@SelamawitA SelamawitA commented Feb 24, 2018

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 pair doesn't have to share one computer. Changes are clearly tracked, for easier reconciliation if there's any discrepancy. It's not necessary to manually manage versions of the file.
Why do you think we chose to make the score method in the Scoring class a class method instead of an instance method? The larger scope of the class method is more appropriate for the Scrabble game because tracking the score transcends any particular instance. The values associated with the letters are constant throughout each instantiation, it is a path to ensuring all objects have access to the same data.
Describe an example of a test you wrote for a nominal case. We created a test to make sure that if 4 tiles were drawn, there were 4 fewer tiles in the bag.
Describe an example of a test you wrote for an edge case. We created a test to make sure that if 100 tiles were drawn, the program would not return any tiles and instead returns an empty array.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? We used an Enumerable to count how many tiles were remaining in our tilebag. It was VERY helpful because it offered a concise way to add up the number of tiles in our bag. We also used an enumerable to traverse through the hash of letters in the TileBag class. It was helpful in reducing the amount of logic we were responsible for in accessing data within the data structure we built.
What was one method you and your pair used to debug code? We used pry to help pick the best enumerable, awesome print with exit to see how our code was progressing, and puts()/print statements.
Is there a specific piece of code you'd like feedback on? The data structure that we used for the tilebag and the draw_tiles methods.
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 talked about how there was a good balance of skills, and an vocal appreciation of the others' contributions. Their was also a good alignment in both curiosity and endurance, resulting in little friction while working through tricky issues.

SelamawitA and others added 30 commits February 20, 2018 15:26
… plays method. Created test for plays and name.
…sly played words if user has not won the game or returning a boolean (false). Included accompying tests.
…est scoring word in an array of played words
SelamawitA and others added 27 commits February 22, 2018 17:41
Added tiles() and play_tiles() method
cleaned up formatting
Cleaned up formatting
added comments
@SelamawitA SelamawitA changed the title Sarah and Selam Amperes Scrabble Sara and Selam Amperes Scrabble Feb 25, 2018
@CheezItMan
Copy link

Scrabble

What We're Looking For

Feature Feedback
General
Answered comprehension questions Check, well done
Both Teammates contributed to the codebase Check both have commits!
Regular Commits with meaningful commit messages Good number of commits!! Mostly great with commit messages, but avoid messages like update player.rb Instead say what you did there.
Scoring class
score method Check
Uses appropriate data structure to store the letter score Your works, but is there a more concise way to store letter scores, maybe using a hash?
Appropriately handles edge cases Handles most edge cases, just missing an empty string.
highest_score_from method One minor bug if the word chosen is invalid.
Appropriately handles edge cases See above
Test implementations for highest_score_from Check, see my one note.
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) Not all, you don't test for invalid words.
TileBag class
Uses a data structure to keep the set of default tiles in the bag Your data structure has one problem, each letter has an equal chance to be drawn despite there being a lot more "E" tiles than "Q". Also you could end up popping an empty array (see my notes).
draw_tiles method uses the parameter to draw a set of random tiles Check
tiles_remaining method returns a count Check
Tests draw_tiles edge cases Check, well done, but you missed the subtle bug above.
Summary Overall nicely done. You hit the learning goals with some minor bugs. See my notes about testing and most especially about TileBag Think about what different kind of structure you could have used.

if array_of_words.size == 0
highest_score = nil
elsif array_of_words.size == 1
highest_score = array_of_words[0]

Choose a reason for hiding this comment

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

What about if the only word is invalid?

end

it 'returns the highest word if there are two words' do
random_arr = ['zzebra','medium']

Choose a reason for hiding this comment

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

You should also vary up the order in these tests so that you can ensure that the order isn't determining the winner (unless it's supposed to).

      random_arr = ['zzebra','medium']
      highest_of_words = Scrabble::Scoring.highest_score_from(random_arr)
      highest_of_words.must_equal 'zzebra'

      random_arr = ['medium','zzebra']
      highest_of_words = Scrabble::Scoring.highest_score_from(random_arr)
      highest_of_words.must_equal 'zzebra'


describe 'total_score' do
it 'Returns a total score to the user' do
player_name = Scrabble::Player.new("Ada")

Choose a reason for hiding this comment

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

You should also test to verify that total_sore doesn't keep going up after the player wins.

end

it 'Returns score of word if < 100 ' do
player_name = Scrabble::Player.new ("Ada")

Choose a reason for hiding this comment

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

What about playing invalid words?

end

it 'Returns false if player has not won' do
player_name = Scrabble::Player.new ("Ada")

Choose a reason for hiding this comment

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

What happens with won? if the player never played a word?

def draw_tiles(tile_bag)
if @players_plaque.length < MAX_NUM_OF_TILES_ALLOWED
add_tiles = MAX_NUM_OF_TILES_ALLOWED - @players_plaque.length
@players_plaque.concat(tile_bag.draw_tiles(add_tiles))

Choose a reason for hiding this comment

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

Very good!

player_status = false
total_score = 0
@array_of_words.each do |word_in_arr|
total_score += Scrabble::Scoring.score(word_in_arr)

Choose a reason for hiding this comment

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

You already have a total_score method inside Player why not use it?


# If the player has over 100 points, returns true, otherwise returns false
def won?
player_case = false

Choose a reason for hiding this comment

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

This variable name really doesn't make sense.


# Returns the highest_scoring_word score
def highest_word_score
return highest_scoring_value = Scrabble::Scoring.score(highest_scoring_word)

Choose a reason for hiding this comment

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

NIIIICE!

random_number = rand(0...tiles_remaining)
@tile_bag.map{|letter_type,letter|
letter.each do |single_letter|
if count == random_number

Choose a reason for hiding this comment

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

The structure you've got here, means that each letter has an equal chance of being drawn. Also after a letter is exhausted you'll continue to pop from the letter array.

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