Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leaves_Ga-Young #41

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Leaves_Ga-Young #41

wants to merge 6 commits into from

Conversation

gyjin
Copy link

@gyjin gyjin commented Aug 26, 2019

Grocery Store

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Response
What is accomplished with raise ArgumentError? The program is prompted that an incorrect input was provided and usually terminates.
Why do you think we made the .all & .find methods class methods? Why not instance methods? We had to make them class methods because we were looking at all the instances in that class, and not at the instance itself. Similarly when finding a particular instance, we had to search through the entire class for that particular 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? This seems like a one-to-many relation. While each customer could have many orders, each order is only tied to one customer. This is similar to Solar System, where the solar system had many planets but each planet was only tied to one solar system.
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? Each customer is listed by its respective ID number and order is not listed in the customer csv file. However, for each order listed, a customer ID number is provided. In the program, the whole instance of a customer is included in the instance of an order. These are different because in the csv file, only an ID number links the two, but in the program, the instances are housed within each other.
Did the presence of automated tests change the way you thought about the problem? How? Yes, it was quite difficult to write this code without a "main.rb". I struggled with not being able to see the output of data or how it was moving or being manipulated. I also found it difficult to determine whether my test code was incorrect or if my code was incorrect. After completion, I now can see how minitest is more efficient than using a "main.rb".

@dHelmgren
Copy link

Grocery Store

What We're Looking For

Feature Feedback
Baseline
Answered comprehension questions On question2, it's slightly incorrect to say 'in that class'. The class method is of the class but it doesn't work in the class the way that standard methods would be said to do. feel free to clarify this 1-1
Used Git Regularly yes
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 the comments to see what can be improved!


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 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