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

Seth Verrill #5

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Conversation

sethverrill
Copy link

Got through the date in iteration 4.
Tests written for #sell, but not enough time to figure out the code.

@@ -0,0 +1 @@
--format documentation
Copy link
Contributor

Choose a reason for hiding this comment

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

Great git commit history!

@price = float_price(attributes[:price])
end

def float_price(string_price)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice helper method!

end

list_items.uniq.sort
end
Copy link
Contributor

Choose a reason for hiding this comment

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

These methods are concise, readable, and follow SRP - nice!

vendor.inventory.each do |item, quantity|
inventory_list[item][:quantity] += quantity
inventory_list[item][:vendors] << vendor
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Great hash manipulation here!


def check_stock(item)
@inventory.has_key?(item) ? @inventory[item] : 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Ternary operator! My personal favorite.

expect(@market.overstocked_items).to eq([@item1])
end

xit 'knows when not to sell item' do
Copy link
Contributor

Choose a reason for hiding this comment

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

Skipped tests? If these are failing, try to get them working for some extra practice.

end
end

describe 'date can be any date but will default to current' do
Copy link
Contributor

Choose a reason for hiding this comment

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

Great test / implementation here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants