Tor Shimizu - GroceryStore - Octos#42
Conversation
Grocery StoreWhat We're Looking For
|
| class Order | ||
| attr_reader :id, :products | ||
| attr_reader :id | ||
| attr_accessor :products |
There was a problem hiding this comment.
I'm curious why you chose to make this an attr_accessor instead of an attr_reader. As far as I can tell, you don't set the value of @products 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.products = products (using = instead of ==), ruby will give you a big error instead of doing the wrong thing.
| products_split = order[1].split(';') # products_split - array of "product:price" | ||
| products_split.each do |mash| | ||
| split = mash.split(':') | ||
| products_hash[split[0]] = split[1].to_f |
There was a problem hiding this comment.
Good work getting this tricky bit of logic sorted out.
| describe "Order.all" do | ||
| it "Returns an array of all orders" do | ||
| # TODO: Your test code here! | ||
| Grocery::Order.all.class.must_equal 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 = Grocery::Order.all
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
end| first_item_id = 1 | ||
| first_product_hash = {"Slivered Almonds"=>22.88, "Wholewheat flour" => 1.93, "Grape Seed Oil" => 74.9} | ||
| Grocery::Order.all.first.id.must_equal first_item_id | ||
| Grocery::Order.all.first.products.must_equal first_product_hash |
There was a problem hiding this comment.
Rather than calling .all several times, best practice is to call it once, save the result in a variable, and then write your expectations against that variable.
The reason why is that loading a file from disk is extremely expensive - hundreds of thousands or millions of times slower than reading from RAM. It doesn't make a big difference yet, but once you start building large pieces of software with large test suites, little changes like this start making a big difference.
See https://gist.github.com/jboner/2841832 for a little more on this.
| total = total.round(2) | ||
| return total | ||
| else | ||
| return total = 0.00 |
There was a problem hiding this comment.
On line 28, you don't need to set the local variable total, since as soon as you return from this method it's gone.
| def initialize(order_id, products, cust_id, fill_status) | ||
| @id = order_id.to_i | ||
| @products = products | ||
| @customer_id = cust_id.to_i |
There was a problem hiding this comment.
Instead of saving @id and @products yourself here, you should call super(order_id, products) and have Order do it for you.
| return nil | ||
| else | ||
| super() | ||
| end |
| end | ||
|
|
||
| #self.find is inherited from the parent class(Order) | ||
|
|
2 participants
Grocery Store
Congratulations! You're submitting your assignment.
Comprehension Questions
raise ArgumentError?.all&.findmethods class methods? Why not instance methods?