-
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
Danielle Cardona #3
base: main
Are you sure you want to change the base?
Conversation
|
||
def initialize(attributes) | ||
@name = attributes[:name] | ||
@price = attributes[:price] |
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.
The interaction patterns asks to clean the string (remove the $) and convert to a float.
attr_reader :name, | ||
:date | ||
|
||
attr_accessor :vendors |
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.
This could probably just be a attr_reader, as this vendor data doesnt need to be updated outside of this class
|
||
if total_inventory[item] | ||
total_inventory[item][:quantity] += quantity | ||
total_inventory[item][:vendors] << vendor unless total_inventory[item][:vendors].include?(vendor) |
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.
There may be an easier way to do it! Might be worth asking chatgpt for some refactor tips.
overstocked_items = [] | ||
|
||
total_inventory.each do |item, info| | ||
if info[:quantity] > 50 && info[:vendors].length > 1 |
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 conditional logic here!
|
||
def potential_revenue | ||
@inventory.sum do |item, quantity| | ||
item.price.delete('$').to_f * quantity |
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.
Cleaning the string down here - nice! Do it in the item class at the beginning :)
it "exists" do | ||
expect(@market).to be_instance_of(Market) | ||
end | ||
|
||
it 'has a name' do | ||
expect(@market.name).to eq("South Pearl Street Farmers Market") | ||
end | ||
|
||
it 'has a date' do | ||
expect(@market.date).to eq("2024/09/16") | ||
end | ||
|
||
it 'starts with an empty array of vendors' do | ||
expect(@market.vendors).to eq([]) | ||
end | ||
end |
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.
Functionality from the initialize method can be consolidated into a single test to save time and lines of code.
def initialize(name) | ||
@name = name | ||
@vendors = [] | ||
@date = Date.today.strftime('%Y/%m/%d') |
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.
OO nice!
@date = Date.today.strftime('%Y/%m/%d') | ||
end | ||
|
||
def add_vendor(vendor_object) |
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.
vendor
is probably an appropriate param name here
No description provided.