Skip to content

Conversation

@CheerOnMars
Copy link

@CheerOnMars CheerOnMars 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's a handy way to group methods that don't have associated states
What is accomplished with raise ArgumentError? It alerts the user that there is an error in the input so they understand why the program may not work as expected
Why do you think we made the .all & .find methods class methods? Why not instance methods? Because they look throughout the whole class, not just at specific instances
Why does it make sense to use inheritance for the online order? Many of the methods from the Order class applied to the OnlineOrder class.
Did the presence of automated tests change the way you thought about the problem? How? It was a nice way to capture all the things to check before getting started, which is a helpful to look at the problem and to be sure that you're testing edge case from the get-go

@tildeee
Copy link

tildeee commented Feb 25, 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, yes, but doesn't meet one large requirement: .all should return an array of instances of Order. This implementation returns an Array of Arrays.
Used CSV library only in Order.all (not in Order.find) x
Used Order.all to get order list in Order.find Great one-line solution :)
Wave 3
All stubbed tests are implemented fully and pass x
Used inheritance in the initialize for online order x, inherited correctly but didn't use super in initialize
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 Again, good one-line solution
Appropriately searches for Customer orders in find_by_customer n/a
Additional Notes

Great job with this project! You're clearly doing a good job with understanding how to make/use local variables, manipulate arrays/hashes, return things, iterate, and call other methods within methods.

However, you missed one large requirement of this project: .all and .find should return an array of Order instances or a single Order instance (respectively). You represented an Order instance through an array of [id, products hash]. Clearly you understand how that array holds the data used to make an array, but we want to see you create an Order with Order.new. I want to make sure you get comfortable/are comfortable with making instances of objects and using them, so let me know.

The tests are slim and accurate though, so I like how the tests look :) Great job with the tests and the project overall

Small note: I think you have something going on with your git configuration because the commits aren't getting tied to your github account, let's figure that out soon.

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