Skip to content

Branches, Kristy - #46

Open
kristyh32 wants to merge 13 commits into
Ada-C12:masterfrom
kristyh32:master
Open

Branches, Kristy #46
kristyh32 wants to merge 13 commits into
Ada-C12:masterfrom
kristyh32:master

Conversation

@kristyh32

Copy link
Copy Markdown

Grocery Store

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Response
What is accomplished with raise ArgumentError? raise argument error allows you to throw an exception when invalid input in entered rather than your program just breaking and giving you an error message. You can also have a display message that could allow the user to fix the input rather than the program just crashing.
Why do you think we made the .all & .find methods class methods? Why not instance methods? .all and .find needed to apply to all instances of customer and order rather than just a particular instance, so they needed to be class methods
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? it is a one to one in that one customer makes a particular order, but it is also one to many in that a customer can make many orders
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? order and customer is kept in the csv on a single line together whereas the program tracks them with instance variables. It makes sense for th CSV to have all the info in a single line, but that would not make any sense for your program to keep the information that way.
Did the presence of automated tests change the way you thought about the problem? How? yes it did. It made it much easier to break the project down into small pieces to tackle one at a time rather than thinking about the whole thing at once and getting overwhelmed.

@kaidamasaki

Copy link
Copy Markdown

Well done!

Comprehension Questions

It seems like you're a little confused about many-to-one vs one-to-one relationships:

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?

When saying whether or not something is one-to-many we look at the entire group. So even though a Customer makes one Order at a time it is set up to handle many orders, so we say that it's one-to-many.

Overall

Well done! Aside from needing more descriptive error messages everything looks great!

Comment thread lib/customer.rb

def self.find(id)

Customer.all.find { |customer| customer.id == id}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good use of find! 🎉

Comment thread lib/order.rb
if valid_statuses.include?(fulfillment_status)
return fulfillment_status
else
raise ArgumentError

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should include a message with your ArgumentErrors so the user knows what is wrong.

Suggested change
raise ArgumentError
raise ArgumentError.new("Invalid fulfillment status: #{fulfillment status}")

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