Ampers: Mary, Alex #13

brownav · 2018-02-24T00:43:05Z

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?	With git you can push and pull on the same code to maintain version control, and work on your own machine while 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 self.highest_score_from uses the Scoring self.class method within it, also the Player class use Scoring self.class as well. It is used across a few classes in the program.
Describe an example of a test you wrote for a nominal case.	To return nil if a word is longer than 7 letters test is pretty straightforward, we fed in an 8 letter word --> Scrabble::Scoring.score('abcdefgh').must_be_nil
Describe an example of a test you wrote for an edge case.	To take in a word for method play, we expect nil if an integer is passed in --> @john.play(20).must_be_nil
Does your code use any methods from the `Enumerable` mixin? If so, where and why was it helpful?	Our highest word score method used the map enumerable on an array of played words to return a new array with the scores of those words. It was helpful in keeping the code compact. We used the resulting array of scores to return the max score.
What was one method you and your pair used to debug code?	We used binding.pry to double-check suspicious values or NilClass errors.
Is there a specific piece of code you'd like feedback on?	Scrabble::Scoring self.highest_score_from method has many returns and might be the most unclear at first glance.
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 our use of good coding language, continual communication, and willingness to experiment with code was accepted graciously and a great benefit to our team.

CheezItMan · 2018-02-26T06:38:53Z

Scrabble

What We're Looking For

Feature	Feedback
General
Answered comprehension questions	Check, an 8-letter word would pretty much be an edge-case. A nominal would be "dog"
Both Teammates contributed to the codebase	Both team put in commits!
Regular Commits with meaningful commit messages	Reasonable number of commits, focus on making the commit messages talk about the functionality added, not "waves".
Scoring class
`score` method	Check, nice work!
Uses appropriate data structure to store the letter score	Nice data structure here!
Appropriately handles edge cases	Check
`highest_score_from` method	Does NOT work, see my inline notes. You also didn't vary the tests to be through enough to catch your bug. Additionally your code did the exact opposite of one of the test descriptions. That would have spotted the bug See my inline notes.
Appropriately handles edge cases	NOPE
Test implementations for `highest_score_from`	You've got some errors, see my inline 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`	You missed a chance to leverage one of your existing methods here. See my inline notes.
Returns `true` or `false` from `won?` method	Check
Tests edge cases on the `play(word)`	No you didn't test what happens if they play more than enough words to win. You also didn't test invalid words, or words played in different order.
TileBag class
Uses a data structure to keep the set of default tiles in the bag	Not a good data structure this time. See my `TileBag` notes.
`draw_tiles` method uses the parameter to draw a set of random tiles	You can draw more of a letter than exist in the bag!
`tiles_remaining` method returns a count	Check
Tests `draw_tiles` edge cases	You should have tested trying to draw more tiles than exist in the bag.
Summary	You've got a lot of problems here. You need to be more through in your testing which would have pointed out some of your bugs. You also needed to think more deeply about the `TileBag` data structure. Lastly your `draw_tiles` method in `Player` misread the requirements. You were to use the parameter not create a new instance. You hit most of the learning goals, but you need to focus more on looking for every possibility in testing.

CheezItMan · 2018-02-26T06:07:06Z

lib/scoring.rb

 module Scrabble
  class Scoring
+    attr_reader :word, :letter
+
+    @@letters_hash = { ["a", "e", "i", "o", "u", "l", "n", "r", "s", "t"] => 1, ["d", "g"] => 2, ["b", "c", "m", "p"] => 3, ["f", "h", "v", "w", "y"] => 4, ["k"] => 5, ["j", "x"] => 8, ["q", "z"] => 10 }


Good data structure for this, but it probably doesn't need to be a class variable. A constant would be a better choice.

CheezItMan · 2018-02-26T06:07:31Z

lib/scoring.rb

 module Scrabble
  class Scoring
+    attr_reader :word, :letter


Why have an attr_reader here? You will never create an instance of Scoring It only has class methods!

CheezItMan · 2018-02-26T06:08:15Z

lib/scoring.rb

+        score << @letter
+      end
+
+      return score.length == 7 ? score.sum + 50 : score.sum


Good use of a ternary

CheezItMan · 2018-02-26T06:10:04Z

specs/scoring_spec.rb

-    it 'returns the highest word if there are two words' do
-    end
+   it 'returns the highest word if there are two words' do
+     Scrabble::Scoring.highest_score_from(["dog", "quail"]).must_equal "quail"


You should also vary the order here to make sure.

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

CheezItMan · 2018-02-26T06:10:19Z

specs/scoring_spec.rb


    it 'if tied, prefer a word with 7 letters' do
+      Scrabble::Scoring.highest_score_from(["aaaaaaa", "ggge"]).must_equal "aaaaaaa"


CheezItMan · 2018-02-26T06:30:22Z

lib/tilebag.rb

+    attr_accessor :tilebag
+
+    def initialize
+      @tilebag = { "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 }


You're giving each letter an equal chance of being drawn with this data structure, but there are 12 "E" tiles and only 1 "J".

CheezItMan · 2018-02-26T06:31:10Z

lib/tilebag.rb

+      hand = []
+      num.times do
+        letter = @tilebag.keys.sample
+        @tilebag[letter] = @tilebag[letter] - 1


This could continually draw down the number of a specific tile into the negatives.

CheezItMan · 2018-02-26T06:31:43Z

lib/tilebag.rb

+        letter = @tilebag.keys.sample
+        @tilebag[letter] = @tilebag[letter] - 1
+        if @tilebag[letter] < 1
+          letter = @tilebag.keys.sample


This doesn't guarantee that a letter out of tiles won't be drawn. You need to rethink your data structure here.

CheezItMan · 2018-02-26T06:33:09Z

specs/tilebag_spec.rb

+    it 'has a collection of all tiles' do
+      new_bag = Scrabble::TileBag.new
+      new_bag.must_be_instance_of Scrabble::TileBag
+      new_bag.tilebag.length.must_equal 26


You don't need to test the internal structure of the Tilebag as a user you don't care that the TileBag has a hash attribute, in fact that shouldn't even be accessible to you. Instead you should test the tiles_remaining method and the other methods listed in the requirements.

CheezItMan · 2018-02-26T06:33:48Z

specs/tilebag_spec.rb

+    end
+
+    it 'removes drawn tiles from tile bag' do
+      old_total = @bag.tilebag.values.sum


You should also test that you can't draw more tiles out of the bag than exist.

brownav and others added 13 commits February 20, 2018 14:42

built letters_hash, testing score

06d58d7

finished self.score, tests run

22a83e5

passes first 3 tests of 'highest_score_from'

4e449cf

test

39a1490

next step return shortest word in case of tie

47b1ac1

fixed merge errors

af74a87

on wave 2, passed initialize test

ba468e7

Defines plays and play methods for class player.

f0d8824

next - work on total_score

5b44c7e

finished wave 2, tests pass

cae8fcd

adding tilebag files

0db556d

adds initialize and draw tiles to tilebag, with tests

a4067b2

all tests pass for each wave

62339f9

CheezItMan reviewed Feb 26, 2018

View reviewed changes

pair programmed on one machine to resolve feedback errors

cc80e61

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ampers: Mary, Alex #13

Ampers: Mary, Alex #13

brownav commented Feb 24, 2018 •

edited

Loading

CheezItMan commented Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018

CheezItMan Feb 26, 2018


		it 'if tied, prefer a word with 7 letters' do
		Scrabble::Scoring.highest_score_from(["aaaaaaa", "ggge"]).must_equal "aaaaaaa"

Ampers: Mary, Alex #13

Are you sure you want to change the base?

Ampers: Mary, Alex #13

Conversation

brownav commented Feb 24, 2018 • edited Loading

Scrabble

Comprehension Questions

CheezItMan commented Feb 26, 2018

Scrabble

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brownav commented Feb 24, 2018 •

edited

Loading