Eric DeShetler #10
Eric DeShetler #10
Conversation
|
|
||
| def initialize(item_info) | ||
| @name = item_info[:name] | ||
| @price = item_info[:price] |
There was a problem hiding this comment.
The interaction pattern asks to clean the string (remove the $) and convert to a float
| def vendors_that_sell(item) | ||
| list = [] | ||
| @vendors.find_all do |vendor| | ||
| list << vendor if vendor.check_stock(item).to_i > 0 |
There was a problem hiding this comment.
Nice logic happening in here
| # find items that are sold by more than one vendor | ||
| temp_items = [] | ||
| # loop through each vendor | ||
| @vendors.each do |vendor| |
There was a problem hiding this comment.
There is an easier way to do this! Might be worth asking chatgpt for some refactor tips.
| items << item[0].name | ||
| end | ||
| end | ||
| items.uniq.sort |
| it 'is and Item' do | ||
| expect(@item1).to be_an_instance_of(Item) | ||
| end | ||
|
|
||
| describe '#initialize' do | ||
| it 'has a name' do | ||
| expect(@item1.name).to eq('Peach') | ||
| end | ||
| it 'has a price' do | ||
| expect(@item1.price).to eq('$0.75') |
There was a problem hiding this comment.
Functionality that. happens in the initialize can be consolidated to the same test, if you want to save some time and some lines.
| @market.add_vendor(@vendor1) | ||
| @market.add_vendor(@vendor2) | ||
| @market.add_vendor(@vendor3) | ||
| expect(@market.overstock_items).to eq([]) |
There was a problem hiding this comment.
The spec asks for #overstocked_items and #sorted_item_list. Try to make you method names match exactly!! On a real team, a developer working on an adjacent feature might have to change their method names if they dont match what is exactly specified.
| @market.add_vendor(@vendor1) | ||
| @market.add_vendor(@vendor2) | ||
| @market.add_vendor(@vendor3) | ||
| expect(@market.vendor_names.length).to eq(3) |
There was a problem hiding this comment.
Instead of checking the length, you should check that vendor_names is an array of the actual names.
| @market.add_vendor(@vendor1) | ||
| @market.add_vendor(@vendor2) | ||
| @market.add_vendor(@vendor3) | ||
| expect(@market.vendors_that_sell(@item1)).to eq([@vendor1, @vendor3]) |
There was a problem hiding this comment.
You could consider adding a test that handles the .stock functionality in isolation. This works though, this is a nice test!
No description provided.