Winifred Irarrazaval - Grocery-Store - Octos #23
Conversation
…nalize arrays of OBJECTS (instances of my class) so I could compare and analize. I used a class in another class
Grocery StoreWhat We're Looking For
|
| module Grocery | ||
| class Order | ||
| attr_reader :id, :products | ||
| attr_accessor :id, :products |
There was a problem hiding this comment.
I'm curious why you chose to make these attr_accessors instead of attr_readers. As far as I can tell, you don't set these values anywhere outside this class, nor should you given the spec.
Leaving it an attr_reader is a safety measure, so that if you accidentally do something like if order.id = 3 (using = instead of ==), ruby will give you a big error instead of doing the wrong thing.
|
|
||
| def add_product (product_name, product_price) | ||
| if @products.keys.include?(product_name) != true | ||
| @products[product_name] = product_price |
There was a problem hiding this comment.
You don't need the explicit != true in this if statement. You could instead say:
if !@products.keys.include?(product_name)
# or
unless @products.keys.include?(product_name)| products_hash = {} | ||
| order[1].split(';').each do |product| | ||
| array_product_price = product.split(":") | ||
| products_hash[array_product_price[0]] = array_product_price[1] |
There was a problem hiding this comment.
Good work getting this tricky bit of logic figured out.
| attr_accessor :customer, :online_order_id, :products, :fulfillment_status | ||
|
|
||
| @customer = Customer.find(id) | ||
| @products = products |
There was a problem hiding this comment.
Looks like you're missing a line here. Adding
def initialize(online_order_id, products, id, fulfillment_status)between lines 9 and 10 made everything work out.
| order_id = order[0].to_i | ||
| products_hash = {} | ||
| id = order[2].to_i | ||
| status = order[3].to_sym |
There was a problem hiding this comment.
It might be wise to call the variable on line 24 customer_id, to make it very clear what it is.
There was a problem hiding this comment.
This comment applies to pretty much anywhere you're using the variable id in this file.
| @customer = Customer.find(id) | ||
| @products = products | ||
| @online_order_id = online_order_id | ||
| @fulfillment_status = fulfillment_status |
There was a problem hiding this comment.
Instead of saving the order id and the product list yourself, you should call super and have the initialize method for Order do that work for you.
| if @fulfillment_status == (:pending || :paid ) | ||
| super | ||
| else | ||
| case @fulfillment_status |
There was a problem hiding this comment.
Could you combine the case and the if into one thing here?
| array = Grocery::Order.all | ||
|
|
||
| array.must_be_instance_of Array | ||
|
|
There was a problem hiding this comment.
You should also check that the elements of the array are instances of class Order. If it gave you a bunch of strings or something like that, that would be a bug!
array.must_be_instance_of Array
# Make sure we got the right number of things from the CSV
array.length.must_equal 100
# Make sure everything has been made into an Order
array.each do |order|
order.must_be_instance_of Grocery::Order
end2 participants
Grocery Store
Congratulations! You're submitting your assignment.
Comprehension Questions
raise ArgumentError?.all&.findmethods class methods? Why not instance methods?