Skip to content

Conversation

@mmlamkin
Copy link

@mmlamkin mmlamkin commented Feb 20, 2018

Grocery Store

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Response
Why is it useful to put classes inside modules? It allows you to group related classes as well as avoid naming conflict
What is accomplished with raise ArgumentError? We didn't use this, but it would identify when you don't enter the necessary number of parameters when using a method
Why do you think we made the .all & .find methods class methods? Why not instance methods? Because we were creating the object instances from a CSV file, the class method could create all the objects and then the instance methods could be used on the objects.
Why does it make sense to use inheritance for the online order? Because it has very similar function to the order class with some adjustments. This way it can inherit the methods already written for Order
Did the presence of automated tests change the way you thought about the problem? How? The automated tests helped me think about what each method should produce and how to tie in the other methods when writing new ones.

@tildeee
Copy link

tildeee commented Feb 26, 2018

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
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
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 x, n/a
Additional Notes

In order to get rake to run, I needed to comment out specs/online_order_spec.rb:10 (online order spec file, line 10) require_relative '../lib/customer' because the file lib/customer wasn't pushed to Github. Be sure that your code can run before you submit. (Also I had to skip the test "OnlineOrder#initialize:Can access Customer object")

Your project looks great overall. It hits all of the requirements and your code looks good, and is clean. Your tests are also slim, thorough, accurate, and concise. I think your project was written pretty well overall.

I think my only comment would be that in your online order spec, you use the local variable onlineorder a lot when I would rather it be named online_order... but literally lol that's the only thing I'd bring up in terms of style.

Great job, Mary!

Grocery::OnlineOrder.find("1").products.must_equal ({"Lobster" => "17.18", "Annatto seed" => "58.38", "Camomile" => "83.21"})

Grocery::OnlineOrder.find("1").customer.must_equal "25"
Grocery::OnlineOrder.find("1").status.must_equal :complete
Copy link

Choose a reason for hiding this comment

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

since this test finds the same order a bunch of times, you probably could just assign a local variable like you do in a lot of the other tests:

order = Grocery::OnlineOrder.find("1")
order.must_be_instance_of Grocery::OnlineOrder
order.products.must_equal ...

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