Skip to content

Kristina Moskalets - Leaves - #50

Open
krismosk wants to merge 9 commits into
Ada-C12:masterfrom
krismosk:master
Open

Kristina Moskalets - Leaves#50
krismosk wants to merge 9 commits into
Ada-C12:masterfrom
krismosk:master

Conversation

@krismosk

Copy link
Copy Markdown

Grocery Store

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Response
What is accomplished with raise ArgumentError? It raises when you call a method with incorrect arguments
Why do you think we made the .all & .find methods class methods? Why not instance methods? Because we want to search through all of the objects in a class, not just one 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? One (order) to many (customer). The solar system was the opposite, many planets 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? The relation is tracked by customer id, it's matched with the .find class method.
Did the presence of automated tests change the way you thought about the problem? How? It informed the way I designed my data structures because I knew what the data types had to look like to pass my tests.

Comment thread lib/order.rb
return product_orders
end

def self.find(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.

This looks good! In case you want something shorter that does exactly what you implemented, you might want to check out: https://ruby-doc.org/core-2.6.1/Enumerable.html#method-i-find

Comment thread test/customer_test.rb
describe "Customer Wave 2" do
describe "Customer.all" do
it "Returns an array of all customers" do
customers = Customer.all

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider putting customers = Customer.all in a before do block so that you can access these customers in future tests rather than calling .all multiple times.

Comment thread test/order_test.rb

end

it "Returns nil for an order that doesn't exist" do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It might be nice to give a name to the value 5747474, like non_extant_order_id = 5747474

@beccaelenzil

Copy link
Copy Markdown

Grocery Store

What We're Looking For

Feature Feedback
Baseline
Answered comprehension questions check -- 1. What happens when an argument error is raised? 3. I think you have it mixed up. It's one customer to many orders.
Used Git Regularly check
Wave 1
All provided tests pass check
Using the appropriate attr_ for instance variables check
Wave 2
All stubbed tests are implemented fully and pass check
Used CSV library only in .all (not in .find) check
Appropriately parses the product data from CSV file in Order.all check
Order.all calls Customer.find to set up the composition relation check
Additional Notes Good job overall! Your code is well-organized and easy to read, and does a good job of solving the problem at hand. I've left a few inline comments for you to review, but it's clear that the learning goals for this assignment were met. Keep up the hard work!

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