Ampers: Nora Peters

[npeters5]

Grocery Store

Congratulations! You're submitting your assignment.

Comprehension Questions

Question	Response
Why is it useful to put classes inside modules?	It's useful to put classes inside modules for namespacing purposes - related classes bundled together inside modules help avoid naming collisions.
What is accomplished with raise ArgumentError?	The 'raise ArgumentError' that I created disallows the user to add a new product to an order if the status of the order is anything other than :pending or :paid.
Why do you think we made the .all & .find methods class methods? Why not instance methods?	We made these methods class methods because they are not tied to a specific instance or state and are more accessible. We used them to read a CSV and create many instances or orders and customers, and we were also able to find a specific order based on passing .find a parameter. I believe an instance methods may have worked in some way but it may have been more cumbersome?
Why does it make sense to use inheritance for the online order?	It makes sense for OnlineOrder to inherit from Order because they had many like behaviors that we didn't want to replicate. Helped to DRY our code.
Did the presence of automated tests change the way you thought about the problem? How?	Absolutely. TDD is a concept that I am eager to master but right now the concept of creating tests before I even write code is still a bit foreign and in some cases for this project I wrote the code before the test. This was good practice in learning the syntax of Minitest and I'm looking forward to getting feedback on how to write better/more useful tests.

[tildeee]

Grocery Store

What We're Looking For

Feature	Feedback
Baseline
Answered comprehension questions	x
Used Git Regularly	x
Wave 1
All provided tests pass	x
Using the appropriate attr_ for instance variables	x, yes, but you could have made :products in attr_reader too
Wave 2
All stubbed tests are implemented fully and pass	x
Appropriately parses the product data from CSV file in Order.all	x
Used CSV library only in Order.all (not in Order.find)	x
Used Order.all to get order list in Order.find	x
Wave 3
All stubbed tests are implemented fully and pass	x
Used inheritance in the initialize for online order	x
Used inheritance for the total method in online order	x
Use CSV library only in OnlineOrder.all	x
Used all to get order list in find	x
Appropriately searches for Customer orders in find_by_customer	n/a
Additional Notes

When I checked out your code and ran the tests, a lot of the tests broke because of lib/online_order.rb:18 (online order file, line 18). If I commented out the line @customer = customer.find(customer_id), the tests ran. After that, I had two test failures in "OnlineOrder::OnlineOrder.find_by_customer", which became optional.

Great job on the project overall! It hits all of the requirements and works as expected.

I only have a few comments on small ways to improve the code stylistically:

• Nice use of the class variable @@all_orders in both Order and OnlineOrder! A lot of people who used a class variable here used it in a way that was buggy, and you avoided that. However, with the way you're using it, you can actually notice... for ever time you use have @@all_orders in any class, you probably could replace it with a local variable all_orders and it would work just the same.
• You do something in your tests that I'll make a comment on the code

Otherwise the tests are great! Overall you pulled out repeated code into before blocks to make your code more DRY.

Good work overall!

[tildeee]

Every time you have Grocery::OnlineOrder.all.first, you're calling and invoking and running Grocery::OnlineOrder.all. Which isn't a bad thing!

But it would be all the same/have less code if you just assigned the result of Grocery::OnlineOrder.all to a local variable at the top of this method:

order = Grocery::OnlineOrder.all.first
order.id.must_equal 1
order.products.must_equal first_row_online_products
order.customer_id.must_equal 25
order.status.must_equal :complete