Nico Vigil PR #5
Conversation
Quell git fire
…urn boats, and check how much revenue it's made
| end | ||
|
|
||
| def price_per_hour | ||
| @cost |
There was a problem hiding this comment.
You could alternatively name @cost @price_per_hour and put it in the attr_reader
| def rent(boat, renter) | ||
| @boats << boat | ||
| @renters << renter | ||
| @boats_and_owners[renter] = boat |
There was a problem hiding this comment.
I like this idea of keeping the owner associated with a boat using a hash
| @revenue += boat.cost | ||
| end | ||
|
|
||
| @boats.map do |boat| |
There was a problem hiding this comment.
This should be an each since you are only using the map to access each boat
| end | ||
|
|
||
| def return(boat) | ||
| delete_this_boat = @boats.find_index do |ship| |
There was a problem hiding this comment.
I think this could be a delete_if
| dock.rent(kayak_1, ringo) | ||
|
|
||
| assert_instance_of Renter, dock.renters.last | ||
| assert_instance_of Boat, dock.boats.last |
There was a problem hiding this comment.
You shouldn't just assert it's a boat, you sure assert the EXACT boat. You have access to it. It's called kayak_1. Same thing for ringo
| kayak_1 = Boat.new(:kayak, 20) | ||
| dock.rent(kayak_1, ringo) | ||
|
|
||
| assert_equal kayak_1, dock.return(kayak_1) |
There was a problem hiding this comment.
This doesn't feel like it's testing what the return method should do. According to what your method does, the return method should remove the boat from the array. This feels like a test for the sake of having a test.
| dock.log_hour | ||
| dock.log_hour | ||
| dock.log_hour # should add 20 to 40 (60) | ||
| dock.log_hour # shouldn't add anything |
| #if I had more time I would have had the methods work off of @boats_and_owners | ||
| #so that the owners were better connected with their boats. The thought crossed | ||
| #my mind but I wanted to make sure I could finish this iteration. | ||
| end |
There was a problem hiding this comment.
The next thing you should do is write another test that the revenue works with multiple boats.
| require "./lib/renter" | ||
| require "./lib/boat" | ||
|
|
||
| class DockTest < Minitest::Test |
There was a problem hiding this comment.
Keep working on your TDD. You are showing improvement. Keep it up!
| @@ -0,0 +1,42 @@ | |||
| require "pry" | |||
| class Dock | |||
There was a problem hiding this comment.
Great job! I like your logic in solving this problem.
Nonstop thrill-ride into the exploration of Ruby