Skip to content

Octos, Jamila#36

Open
Jcornick21 wants to merge 6 commits into
Ada-C9:masterfrom
Jcornick21:master
Open

Octos, Jamila#36
Jcornick21 wants to merge 6 commits into
Ada-C9:masterfrom
Jcornick21:master

Conversation

@Jcornick21

Copy link
Copy Markdown

Grocery Store

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Response
Why is it useful to put classes inside modules? not sure
What is accomplished with raise ArgumentError? didnt get to this
Why do you think we made the .all & .find methods class methods? Why not instance methods? don't know
Why does it make sense to use inheritance for the online order? don't know
Did the presence of automated tests change the way you thought about the problem? How? don't know

@CheezItMan

CheezItMan commented Feb 23, 2018

Copy link
Copy Markdown

Grocery Store

What We're Looking For

Feature Feedback
Baseline
Answered comprehension questions NOPE Check in with Dan or myself, or Charles. These are questions we need to help you answer.
Used Git Regularly You should commit a lot more often. Commit to git! Your commit messages are pretty good however.
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 MISSING
Appropriately parses the product data from CSV file in Order.all Yes, although it's more complicated than it needs to be. See my in-code notes.
Used CSV library only in Order.all (not in Order.find) Check
Used Order.all to get order list in Order.find Order.all is used, but the method is not functioning right now. Check out my in-code notes.
Wave 3
All stubbed tests are implemented fully and pass MISSING
Used inheritance in the initialize for online order MISSING
Used inheritance for the total method in online order MISSING
Use CSV library only in OnlineOrder.all MISSING
Used all to get order list in find MISSING
Appropriately searches for Customer orders in find_by_customer MISSING
Additional Notes A lot of Order is good. Did you forget to commit your testing files? I'm not seeing them. If you're having trouble let Dan know or let me know and we can look at places where you need things explained differently or some help. There's a lot missing here and on this project the submission isn't getting to the learning goals.

Comment thread lib/order.rb
require "pry"

# access CSV and other type use require relative
FILE_NAME = "../support/orders.csv"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. Good that you're using a constant!
  2. The path is off. You should write it from the perspective of whomever is running the app. Users should run rake from the project's home folder, so the path to the CSV from the home folder is support/orders.csv

So this should be:

FILENAME = "support/orders.csv"

Comment thread lib/order.rb
@products[product_name]= product_price
return true
end
return products

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 return will never be reached

Comment thread lib/order.rb
end

def self.all
all_orders = []

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 method is a little overly complicated.

CSV.open.each will loop through each row, and below where you have all_orders.each will also loop through each row. So you could merge the two, eliminating the CSV.open loop and replace the top line of the 2nd with:

CSV.open(FILE_NAME, 'r').each do |order|

However what' you've got works and that's good.

Comment thread lib/order.rb
end

def self.find(id)
Grocery::Order.all.each do |order|

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 isn't working. It's good that you're using Order.all to get the list, but instead of returning order.products it should just return order

If the loop finishes without finding an order it should return nil

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