Kiera - Grocery Store - Octos #25
Conversation
…r of products, is added to collection of products tests
… the list and replaced the entire list with snickers, 2.50
…e final hash of items is assigne to the products variable.
…quiteite returning the correct information.
…ders for self.all
…fter adding module to online_order.rb
Grocery StoreWhat We're Looking For
This is a decent start, and I see a lot of things I like. The tests you wrote for wave 2 look great aside from the product list, and it feels like you were on the right track with inheritance for wave 3. However, like we discussed on Slack it seems like you're still having a lot of trouble working through problems, going from an idea of what you want to working Ruby code. In this project, the result is that much of your logic for waves 1 and 2 doesn't really work, and you didn't have the time to address wave 3 fully. Given all that, I think it would be wise to match you with a tutor for some extra one-on-one support. Hopefully that will help give you the boost you need to really feel confident on these projects. I'll speak with Charles about it, and you can expect him to reach out to you soon. |
| require 'awesome_print' | ||
| require 'pry' | ||
| require 'csv' | ||
|
|
There was a problem hiding this comment.
To get access to Order in this fill, you need to put a require_relative 'order' here.
| else | ||
| return true | ||
| @products[:product_name] = product_price | ||
| before_count = @products.count |
There was a problem hiding this comment.
Putting a return on line 31 prevents the rest of the method from running! The product is never added to the order.
In fact, because you've got the if statement above, we can shorten this else statement to
else
@products[:product_name] = product_price
return true
endNo need to worry about the counts.
| products = line[1].split(";")#[1] | ||
| products.each do |items|# take the string split at the : and assign them to key value pairs | ||
| # ap products | ||
| products = items.split(":") |
There was a problem hiding this comment.
You should be using a different name for the result of this split. Calling it products will override the products you've defined outside.
| product_price = items[1].to_i | ||
|
|
||
| # products[product_name] = product_price | ||
| orders << Order.new(id, products) |
There was a problem hiding this comment.
I'm not sure that this logic will work for .all. Here's what I'm seeing:
itemsis a single value, a string that looks like"banana:3.99"productsis an array with two values that looks like['banana', 3.99]- On lines 62 and 63, you split the name and the price into separate local variables, but these are never used
- On line 66, you make an
Orderinside the inner loop. That means you'll create an order for every product-price pair, for every line in the CSV file, more than we want.
You are, however, quite close. If you were to make the following changes, this code would look good:
- Between line 55 and 56, create a new local variable
products_hash = {} - Instead of creating an order for each pair in the list, add the pair to the
products_hash- Replace line 66 with
products_hash[product_name] = product_price
- Replace line 66 with
- After the inner loop (between 67 and 68), create a new
Orderfrom the id and theproducts_hashand add it to the list
| if order.id == id | ||
| return order | ||
| else | ||
| return nil |
There was a problem hiding this comment.
I don't think this return nil makes sense inside of the loop. That means as soon as it hits an order that's not the one we're looking for, it'll return! Searching for anything other than the first order wouldn't work.
Instead, you could put the return nil after the loop. Your code then would effectively say, "If in all of that looping I didn't find the matching order and return it already, I will return nil now".
|
|
||
| FILENAME = "support/orders.csv" | ||
|
|
||
|
|
There was a problem hiding this comment.
Good use of a constant to hold the file name
|
|
||
| def initialize(id, products, status = :pending) | ||
| super(id, products) | ||
| @customer_id = customer_id |
There was a problem hiding this comment.
Good use of super here to keep this DRY.
| # same as order except will add $10 shipping fee | ||
| def total | ||
| super() | ||
| return sum += 10 |
There was a problem hiding this comment.
In order for this to work, you'll need to keep track of the value you get from super. Something like:
def total
sum = super()
return sum + 10
end| super() | ||
| unless status == pending || status == paid | ||
| raise ArgumentError, "INVALID STATUS" | ||
| end |
There was a problem hiding this comment.
You'll probably want to make this check before the call to super.
| # Arrange - feed information for method to work with | ||
|
|
||
| products_list = ["Slivered Almonds", "22.88"], ["Wholewheat flour", "1.93"],["Grape Seed Oil" , "74.9"] | ||
|
|
There was a problem hiding this comment.
The product lists in orders you get from .all should have the same form as the hashes we saw in previous tests.
2 participants
Grocery Store
Congratulations! You're submitting your assignment.
Comprehension Questions
raise ArgumentError?.all&.findmethods class methods? Why not instance methods?Instead, class methods are useful when a behavior is associated with the whole class, instead of one particular instance. If we made
.allor.findinstance methods, then you would have to have an instance ofOrderto get all the orders, which doesn't make much sense. With a class method, all you need is to know that there is a thing calledOrder, you don't already have to have one.