Ampers: Alex#29
Open
brownav wants to merge 7 commits into
Open
Conversation
Grocery StoreWhat We're Looking For
|
CheezItMan
reviewed
Feb 22, 2018
| module Grocery | ||
| class OnlineOrder < Order | ||
|
|
||
| attr_reader :id, :products, :customer_id, :status |
There was a problem hiding this comment.
You can keep the attr_reader from Order and just have here:
attr_reader :customer_id, :status
| end | ||
|
|
||
| def total | ||
| if super == 0 |
| if @status == "pending" || @status == "paid" | ||
| return super | ||
| else | ||
| return ArgumentError |
There was a problem hiding this comment.
I think this should be: return ArgumentError.new "Unable to add a product due to status #{@status}"
However good on you for using ArgumentError
| end | ||
|
|
||
| def self.all | ||
| @@all_online_orders = [] |
There was a problem hiding this comment.
Since you're only using the class variable in this method, just use a local variable
|
|
||
| def self.find(id) | ||
| if id <= self.all.length | ||
| specific_online_order = self.all[id - 1] |
There was a problem hiding this comment.
You shouldn't depend on the file being sorted. If the file was changed in Excel or something...
| # Feel free to split this into multiple tests if needed | ||
| end | ||
| end | ||
| array_of_customers = Grocery::Customer.all |
| # TODO: Your test code here! | ||
| it "Returns accurate information about the first customer" do | ||
| array_of_customers = Grocery::Customer.all | ||
| first_customer_id = array_of_customers[0][0] |
There was a problem hiding this comment.
This should be expecting:
array_of_customers[0].must_be_instance_of Order
array_of_customers[0].id.must_equal 1etc
Instead your test code is expecting Customer.all to return an array of arrays. It should return an array of Customer objects.
This kind of thing applies for all your tests.
| it "Can find the first customer from the CSV" do | ||
| first_customer_info = Grocery::Customer.find(1) | ||
|
|
||
| first_customer_info.must_equal Grocery::Customer.all[0] |
There was a problem hiding this comment.
You shouldn't test find against all since one depends on the other (circular testing).
Instead you should
first_customer = Grocery::Customer.find(1)
first_customer.id.must_equal 1
first_customer.email.must_equal .....| describe "#add_product" do | ||
| it "Does not permit action for processing, shipped or completed statuses" do | ||
| # TODO: Your test code here! | ||
| assert_raises ArgumentError do |
There was a problem hiding this comment.
To do this spec-style you can:
proc {
@online_order.add_product
}.must_raise ArgumentError| array_of_orders = Grocery::OnlineOrder.all | ||
|
|
||
| array_of_orders.must_be_kind_of Array | ||
| array_of_orders.length.must_equal 100 |
There was a problem hiding this comment.
You should also check to see if everything is an OnlineOrder.
array_of_orders.all? do |order|
order.class == Grocery::OnlineOrder
end
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Grocery Store
Congratulations! You're submitting your assignment.
Comprehension Questions
raise ArgumentError?.all&.findmethods class methods? Why not instance methods?