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

&&: Mariko and Monalisa's Scrabble #7

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

Conversation

MonalisaC
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? We can both see and make changes to the same document.
Why do you think we chose to make the score method in the Scoring class a class method instead of an instance method? So that it can be used in all instance of the class.
Describe an example of a test you wrote for a nominal case. We wrote a test for draw tiles to check if it returns a collection of random tiles.
Describe an example of a test you wrote for an edge case. For Scoring.score we did an edge case like returns nil for string containing bad characters .
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? In Scoring#highest_score_from we used some enumerables like map, max it is more concise and short.
What was one method you and your pair used to debug code? Used puts statement to debug.
Is there a specific piece of code you'd like feedback on? More examples of edge cases
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? Trying to figure out the best way to communicate with each other. Different processing process while working in pair.

marikoja and others added 30 commits February 20, 2018 14:28
@CheezItMan
Copy link

Scrabble

What We're Looking For

Feature Feedback
General
Answered comprehension questions Check, for the class methods we made them such because the methods aren't tied to a particular instance of a class and they don't really fit a Player action.
Both Teammates contributed to the codebase Both teammates contributed commits.
Regular Commits with meaningful commit messages Good number of commits and good commit mesages
Scoring class
score method Check
Uses appropriate data structure to store the letter score Check
Appropriately handles edge cases Check
highest_score_from method Check, nicely done.
Appropriately handles edge cases Check
Test implementations for highest_score_from See my notes
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, nicely done
Returns true or false from won? method Check
Tests edge cases on the play(word) Check, see my notes however.
TileBag class
Uses a data structure to keep the set of default tiles in the bag Well done
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 Missing some edge cases
Summary Well done, you used a lot of enumerables, and caught most of the tricky things with Scrabble. See my notes, but overall well done!

return total
end

def self.pick_winner(highest_scoring_words)

Choose a reason for hiding this comment

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

Nice a helper method! Good use of min_by.

return highest_scoring_words.min_by { |word| word.length }
end

def self.highest_scoring_words(array_of_words)

Choose a reason for hiding this comment

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

Good use of Enumerables.


def self.highest_score_from(array_of_words)
def self.highest_score_from(array_of_words)

Choose a reason for hiding this comment

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

Very neat how you broke this down.

# letters is a array of upcase letters
letters = word.upcase.chars.to_a

letters.map{ | letter |

Choose a reason for hiding this comment

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

You don't need map here, map is used to convert an array to something else. If you wanted to use map you could:

scores = letters.map do |letter|
   CHART[letter.toupcase]
end
return scores.sum

end

it 'returns the only word in a length-1 array' do
Scrabble::Scoring.highest_score_from(['BANJO']).must_equal 'BANJO'
end

it 'returns the highest word if there are two words' do

Choose a reason for hiding this comment

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

Good job swapping the order!

end


it 'Adds the input word to the plays Array' do

Choose a reason for hiding this comment

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

What about playing words, after the player has won?


it "Fills tiles array, after player uses the tiles" do
player.draw_tiles(tilebag)
word = player.tiles.join

Choose a reason for hiding this comment

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

Clever

class TileBag
attr_reader :tiles
def initialize
@tiles = ["A"] * 9 + ["B"] * 2 + ["C"] * 2 + ["D"] * 4 + ["E"] * 12 + ["F"] * 2 + ["G"] * 3 + ["H"] * 2 + ["I"] * 9 + ["J"] * 1 +["K"] * 1 + ["L"] * 4 + ["M"] * 2 + ["N"] * 6 + ["O"] * 8 + ["P"] * 2 + ["Q"] * 1 +["R"] * 6 + ["S"] * 4 + ["T"] * 6 + ["U"] * 4 + ["V"] * 2 + ["W"] *2 +["X"] * 1 + ["Y"] * 2 + ["Z"] * 1

Choose a reason for hiding this comment

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

Awesome data structure here.


describe 'TileBag' do

let(:tilebag) {Scrabble::TileBag.new}

Choose a reason for hiding this comment

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

Nice use of a let

tilebag.draw_tiles(0).length.must_equal 0

end
it "removes the tiles from the default set" do

Choose a reason for hiding this comment

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

An edge-case would be drawing more tiles than exist in the bag.

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