-
Notifications
You must be signed in to change notification settings - Fork 14
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
Jeremiah Ross #6
base: main
Are you sure you want to change the base?
Conversation
started working on iteration 4 sate but ran out of time
@@ -1,3 +1,15 @@ | |||
class Item |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next time I would complete all of iteration 3 before moving to iteration 4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great git commit history.
end | ||
|
||
def price | ||
@price_str.delete("$").to_f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work cleaning the string!
end | ||
|
||
def date | ||
@date.strftime('%d/%m/%Y') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done! There are other ways to do this, too.
end | ||
|
||
it 'can have a date in the past a future' do | ||
# allow(@market3).to receive(:initialize).and_return(:) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mocks and stubs would have helped with this test!
expect(@vendor1.inventory).to eq({@peach => 55}) | ||
end | ||
|
||
it 'can have multiplle items in stock' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling mistake
|
||
RSpec.describe Market do | ||
before(:each) do | ||
@peach = Item.new({name: 'Peach', price: "$0.75"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test setup here
@vendor1 = Vendor.new("Rocky Mountain Fresh") | ||
@vendor2 = Vendor.new("Bob's Fish Market") | ||
|
||
@vendor1.stock(@peach, 25) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stocking items might not be a part of your setup, but it works here! Just something to think about - I'd have liked to see the stock method tested in isolation.
Completed Iteration 1 & 2, did Market #overstocked_items and Market #sorted_item_list for iteration 3
Started date for iteration 4 but ran out of time