Skip to content

Conversation

emilyvomacka
Copy link

Grocery Store

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Response
What is accomplished with raise ArgumentError? Prevents the method from crashing when invalid input is entered. Alerts the user that invalid input is the source of the problem.
Why do you think we made the .all & .find methods class methods? Why not instance methods? Because they involve many instances, not just work within a single instance.
Think about the relation between Order and Customer. Is this relation one-to-one, one-to-many, or something else? How does that compare to the Solar System project? Order and Customer have a 1:1 relationship. Solar System and Planet have a 1:many relationship.
How is the relation between Order and Customer tracked in the CSV file? How is it tracked in your program? Why might these be different? The CSV tracks each order's customer by only including the customer's ID. For more customer info, you must enter the ID into the customer.find function to pull up that customer's full profile. In my program, the Order object is composed (partially) of a Customer object, through which all customer info can be accessed more easily (ex. order.customer.address). This would appear to be one of the strengths of OOP.
Did the presence of automated tests change the way you thought about the problem? How? Helped me get a better, more concrete answer to "is this right" than running a bunch of hand-created tests in the terminal each time I changed my method. Also made it easier to edit parts of a class and then quickly check to see if anything had broken as a result of my changes.

@dHelmgren
Copy link

Grocery Store

What We're Looking For

Feature Feedback
Baseline
Answered comprehension questions Throwing any sort of error actually does cause a crash, but what we get out of it is that we tell other developers that the method was used wrong.
Used Git Regularly I would expect to see more commits on a project of this size.
Wave 1
All provided tests pass yes
Using the appropriate attr_ for instance variables mostly see comment
Wave 2
All stubbed tests are implemented fully and pass yes
Used CSV library only in .all (not in .find) yes
Appropriately parses the product data from CSV file in Order.all yes
Order.all calls Customer.find to set up the composition relation yes
Additional Notes Solid work! Check my comments to see what can be improved in the future!

#returns a collection of Customer instances, representing all of the Customers described in the CSV file
def self.all
customer_array = []
filename = "/Users/emilyvomacka/Documents/Ada/week_three/grocery-store/data/customers.csv"

Choose a reason for hiding this comment

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

You use the absolute path here, which causes this to crash on my machine. Don't do that! I don't want to change your code. a fix:

Suggested change
filename = "/Users/emilyvomacka/Documents/Ada/week_three/grocery-store/data/customers.csv"
filename = "./data/customers.csv"


class Order
attr_reader :id
attr_accessor :products, :customer, :fulfillment_status

Choose a reason for hiding this comment

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

do all of these need to be accessors?

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