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

Wave 3 complete #31

Open
wants to merge 5 commits into
base: satald/master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added .DS_Store
Binary file not shown.
Binary file added lib/.DS_Store
Binary file not shown.
3 changes: 0 additions & 3 deletions lib/highest_score.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ def self.highest_score_from(array_of_words)
wordscore_hash[w] = score

Choose a reason for hiding this comment

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

This is a great use of a hash in order to store the scores

end

#find max value of a value in a hash
# wordscore_hash.max[0]

#returns all keys with max value
top_scoring_words_hash = {}
wordscore_hash.each do |k, v|

Choose a reason for hiding this comment

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

A more appropriate enumerable method to complete this logic could be find_all

Choose a reason for hiding this comment

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

Watch you indentation within the if statement on line 36 below

Choose a reason for hiding this comment

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

Overall, I'd recommend using full key and value variable names, rather than the single-letter variables, k and v

Expand Down
9 changes: 8 additions & 1 deletion lib/player.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
require 'pry'
require "./lib/score.rb"
require "./lib/highest_score.rb"
require "./lib/tilebag.rb"

module Scrabble

class Player
attr_accessor :name, :words_played, :player_score
attr_accessor :name, :words_played, :player_score, :tiles

def initialize(name)
@name = name.capitalize
@words_played = []
@player_score = 0
@tiles = []
end

def play_word(word)

Choose a reason for hiding this comment

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

Nice use of the won? method to determine whether or not the user has won

Choose a reason for hiding this comment

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

You could simplify this won? method to use a single-line conditional like
return true if @player_score > 100

Expand Down Expand Up @@ -44,5 +46,10 @@ def highest_scoring_word
def highest_word_score
ScrabbleGame.score(highest_scoring_word)
end

def draw_tiles(tilebag)
num = 7 - @tiles.length
@tiles += (tilebag.draw_tiles(num))
end
end
end
8 changes: 4 additions & 4 deletions lib/score.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ def self.score(word)
word_score += value[letter]

Choose a reason for hiding this comment

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

The value hash above will be re-created every time the score method is called which seems redundant. I think it would be better if this hash was stored as a constant because then it would not be recreated each time the method was called.

end

if letters.length == 8
word_score += 50
puts "BONUS! You used all seven letters! You get 50 extra points!"
end
if letters.length == 8
word_score += 50

Choose a reason for hiding this comment

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

Watch your indentation here - should be indented one additional level

puts "BONUS! You used all seven letters! You get 50 extra points!"
end

return word_score
end
Expand Down
24 changes: 10 additions & 14 deletions lib/tilebag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,6 @@

module Scrabble

# DONE! Create a TileBag class with a minimum of 5 specs. It should have the following class and instance methods:
# DONE! #initialize Called when you use TileBag.new, sets up an instance with a collection of default tiles
# #draw_tiles(num) returns num number of random tiles, removes the tiles from the default set.
# #tiles_remaining returns the number of tiles remaining in the bag
# Create specs for (minimum 2) and add to the Player class the following instance methods:
#
# #tiles a collection of letters that the player can play (max 7)
# #draw_tiles(tile_bag) fills tiles array until it has 7 letters from the given tile bag

class TileBag

attr_accessor :quantity
Expand Down Expand Up @@ -51,18 +42,23 @@ def initialize
# turn @quantity into an array, shuffle, pop, and return x amount of letters

def draw_tiles(num)
if num > 7

Choose a reason for hiding this comment

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

What happens if the number of tiles left is less than 7?

return []
else
letters_array = []
num.times do
letters_array.push(@quantity.keys.shuffle.pop)
end

letters_array.each do |letter|
quantity.find

@quantity[letter] -= 1
end
return letters_array
end
end

return letters_array

def tiles_remaining
sum = @quantity.values.inject(0) {|sum, value| sum + value}
sum

Choose a reason for hiding this comment

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

The line above would be sufficient in returning the sum variable value. I would recommend that if you want/need to do an explicit return, use the return keyword to make it a bit more clear

end
end
end
Binary file added spec/.DS_Store
Binary file not shown.
37 changes: 37 additions & 0 deletions spec/player_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
describe Scrabble::Player do
before :each do
@bob = Scrabble::Player.new("Bob")
@tilebag = Scrabble::TileBag.new

Choose a reason for hiding this comment

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

It seems like this @tilebag instance variable was unnecessarily added for all of your existing tests. Think about the fact that this variable would then be created for each of your existing tests where you aren't using it. I would recommend having another section of your RSpec tests that initialized this variable for only the tests where it is needed.

end

describe ".new" do
Expand Down Expand Up @@ -99,5 +100,41 @@
end
end

describe "tiles" do
it "Tiles is an instance variable is created on player instatiation" do
expect(@bob.tiles).to eq []
end
end
#
describe "draw_tiles" do
before :each do
@bob.draw_tiles(@tilebag)
end
it "Fills player's tiles array with the given number of tiles, starting from an empty @tiles" do
expect(@bob.tiles.length).to eq 7
end
end

describe "draw_tiles" do
before :each do
@bob.tiles = ["x", "x"]
@bob.draw_tiles(@tilebag)
end
it "Fills player's tiles array with the given number of tiles, starting from a non-empty @tile" do
expect(@bob.tiles.length).to eq 7
end
it "Removes the same number of tiles from the tile bag" do
expect(@tilebag.tiles_remaining).to eq 93
end
end

describe "draw_tiles" do
before :each do
@bob.tiles = ["x", "x", "x", "x", "x", "x", "x"]
@bob.draw_tiles(@tilebag)
end
it "@tiles remains the same if it is already full" do
expect(@bob.tiles).to eq ["x", "x", "x", "x", "x", "x", "x"]
end
end
end
59 changes: 28 additions & 31 deletions spec/score_spec.rb
Original file line number Diff line number Diff line change
@@ -1,37 +1,34 @@
require "./lib/score.rb"

# describe Word do
# @word = Scrabblegame.new
# end

describe "#word as parameter" do
it "checks if the input is a string" do
word = "test"
#.to be String/Fixnum
expect(word).to be_an_instance_of String
describe Scrabble::ScrabbleGame do
describe "#word as parameter" do
it "checks if the input is a string" do
word = "test"
expect(word).to be_an_instance_of String

Choose a reason for hiding this comment

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

This test isn't really testing that the parameter can be accepted as a string, but rather it is just testing the string class in ruby - it is not referencing the code you've written at all

end
end
end

describe "score" do
it "checks the value of the word 'puppy'" do
word = "puppy"
expect(Scrabble::Scrabblegame.score(word)).to eq 14
end
it "checks the value of the word 'huzzah'" do
word = "huzzah"
expect(Scrabble::Scrabblegame.score(word)).to eq 30
end
it "checks the value of the word 'cat'" do
word = "cat"
expect(Scrabble::Scrabblegame::score(word)).to eq 5
end
it "checks the value of nothing" do
word = ""
expect(Scrabble::Scrabblegame::score(word)).to eq 0
end
it "checks that 50 points are added to an 8 letter word" do
word = "flapjack"
expect(Scrabble::Scrabblegame::score(word)).to eq 76
end
describe "score" do
it "checks the value of the word 'puppy'" do
word = "puppy"
expect(Scrabble::ScrabbleGame.score(word)).to eq 14
end
it "checks the value of the word 'huzzah'" do
word = "huzzah"
expect(Scrabble::ScrabbleGame.score(word)).to eq 30
end
it "checks the value of the word 'cat'" do
word = "cat"
expect(Scrabble::ScrabbleGame::score(word)).to eq 5
end
it "checks the value of nothing" do
word = ""
expect(Scrabble::ScrabbleGame::score(word)).to eq 0
end
it "checks that 50 points are added to an 8 letter word" do
word = "flapjack"
expect(Scrabble::ScrabbleGame::score(word)).to eq 76
end

end
end
37 changes: 25 additions & 12 deletions spec/tilebag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,33 @@
@tilebag = Scrabble::TileBag.new
end

describe ".new" do
it "creates a new instance of a tile bag" do
expect(@tilebag).to be_an_instance_of Scrabble::TileBag
end
describe ".new" do
it "creates a new instance of a tile bag" do
expect(@tilebag).to be_an_instance_of Scrabble::TileBag
end
end

describe "draw_tilebag(num)" do
it "randomly grabs (num) number of tiles" do
expect((@tilebag.draw_tiles(3)).length).to eq 3
end

describe "draw_tilebag(num)" do
before :each do
@tilebag.draw_tiles(4)
end
it "removes tiles from the tilebag" do
expect(@tilebag.tiles_remaining).to eq 94
end
it "grabs (num) number of tiles" do
expect((@tilebag.draw_tiles(3)).length).to eq 3
end
it "does not likely draw the same letter array twice" do
expect(@tilebag.draw_tiles(6)).not_to eq(@tilebag.draw_tiles(6))
end
it "does not allow user to draw over 7 tiles" do
expect(@tilebag.draw_tiles(8)).to eq []
end
end




describe "tiles_remaining" do
it "returns the total number of tiles that remain after a Player" do
expect(@tilebag.tiles_remaining).to eq 98
end
end
end