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

&&: Aruna & Kate #10

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

&&: Aruna & Kate #10

wants to merge 32 commits into from

Conversation

Oh-KPond
Copy link

@Oh-KPond Oh-KPond commented Feb 23, 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? It was helpful for creating sandboxes for our code, as well as being able to use our own environments/computers. It helped if we ever wanted to go back to a previous state.
Why do you think we chose to make the score method in the Scoring class a class method instead of an instance method? A class method can be used for all instances instead of just one instance or "player". Since a game has many players, the score should be accessible to all of those players.
Describe an example of a test you wrote for a nominal case. The nominal cases, for example, were those that checked to see if the player responded to play or name.
Describe an example of a test you wrote for an edge case. We did write an edge case for the player play method to make sure that a player could not enter a word that includes special characters.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? We did use max_by and min_by to help with the code that decided a winner when there was a tie. It was helpful because it cut down on code that needed to be changed, and made the code base less bulky.
What was one method you and your pair used to debug code? We used the ruby gem Awesome Print a lot to see what our variable outputs were, and Minitest was great to see where errors were happening.
Is there a specific piece of code you'd like feedback on? We would like to see feedback on testing and how that can be done better.
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? When we were thinking about the code, but not pair programming, it was great to use Slack as a way to communicate and give feedback about our ideas. We really thought it was important that we were open with each other throughout the program so that that we could remind ourselves of how we had originally planned to grow.

aruna79 and others added 30 commits February 20, 2018 15:23
creates test and code for '#draw_tiles'
refactors tilebag init to turn it into a collection of tiles
redesigns #draw_tiles and #tiles_remaining tests and code
@CheezItMan
Copy link

Scrabble

What We're Looking For

Feature Feedback
General
Answered comprehension questions Check, I'm glad it sounds like you worked well together.
Both Teammates contributed to the codebase Check
Regular Commits with meaningful commit messages Check, good commit messages and regular commits
Scoring class
score method Check
Uses appropriate data structure to store the letter score There is a more efficient way to do this, consider a hash.
Appropriately handles edge cases Check
highest_score_from method Check
Appropriately handles edge cases You need to consider invalid words
Test implementations for highest_score_from See my in-code notes, you also need to write tests to ensure the order isn't causing a word to win despite another rule.
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, but see my notes in your code.
Tests edge cases on the play(word) Some
TileBag class
Uses a data structure to keep the set of default tiles in the bag Check, good data structure, but poor choice in how you remove tiles from the bag.
draw_tiles method uses the parameter to draw a set of random tiles You are calling draw_tiles as if it's a class method and not an instance method. See my incode notes.
tiles_remaining method returns a count NOT WORKING
Tests draw_tiles edge cases You tried, but they're failing.
Summary You hit most of the learning goals. You need to look more closely at your TileBag class and how tiles are being drawn, you're on the right track, but need more testing and more work on draw_tiles

end

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

Scrabble::Scoring.highest_score_from(["dog", "rooster"]).must_equal "rooster"

Choose a reason for hiding this comment

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

In these tests you should also vary up the order, just to make sure the order of the elements isn't determining the answer.

      Scrabble::Scoring.highest_score_from(["dog", "rooster"]).must_equal "rooster"
     Scrabble::Scoring.highest_score_from(["rooster", "dog"]).must_equal "rooster"

Also what about testing a list of words with an invalid word in the mix.

score = 0
letters_array.each do |letter|
case letter
when "a" ,"e","i","o", "u", "l", "n", "r" ,"s","t"

Choose a reason for hiding this comment

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

This works, but is there a more compact way to do this using a hash?

# Arrange
# Act
# Assert
Scrabble::Scoring.highest_score_from([]).must_be_nil

Choose a reason for hiding this comment

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

Good, now what about an invalid word in the list?

module Scrabble
class Player
attr_reader :name, :plays, :total
attr_accessor :tiles

Choose a reason for hiding this comment

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

This should not be an accessor.

@name = name
@plays = Array.new
@total = 0
@tilebag = Scrabble::Tilebag.new

Choose a reason for hiding this comment

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

Each player should NOT receive their own TileBag. Instead use the parameter provided to draw_tiles.

it "returns true if total_score greater than or equal to 100" do

@player.play("qqqq")
@player.total_score

Choose a reason for hiding this comment

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

won? shouldn't fail if the user doesn't call total_score first.


describe "# draw_tiles method" do
it "Should be a collection of letters" do

Choose a reason for hiding this comment

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

You're not testing the draw_tiles method!

end

def tiles_remaining
@tiles.length - @drawn_tiles.length

Choose a reason for hiding this comment

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

This isn't working


def draw_tiles(tilebag)
num = 7 - @tiles.length
replace_tiles = Scrabble::Tilebag.draw_tiles(num)

Choose a reason for hiding this comment

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

You're calling draw_tiles like a class method, not an instance method of tilebag

Should be:

tilebag.draw_tiles(num)


describe '#draw_tiles(num)' do
it "returns an array of tiles" do
@tilebag.draw_tiles(9)

Choose a reason for hiding this comment

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

This test is failing

if array_of_words == []
return nil
elsif array_of_words.length == 1
winning_word_array << array_of_words

Choose a reason for hiding this comment

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

If you're just going to return 1 word, why push array_of_words into winning_word_array instead return array_of_words[0]

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