NYU-DevOps-Charlie-CSCI-GA-3033-013 / products

Product Team for CSCI-GA.3033-013
http://nyu-lab-bluemix-charlie-products.mybluemix.net/
1 stars 1 forks source link

added query price function #23

Closed NicholasHyland closed 7 years ago

NicholasHyland commented 7 years ago

Added two price query functions:

Also modified all other functions and test cases to reflect the addition of a new PRICE column into the data schema

ephraimrosenfeld commented 7 years ago

My understanding of the functionality was a bit different.

I was under the assumption that we would augment to the list_products() method by adding min and/or max price attributes, e.g. GET localhost:5000/product?min=10.5&max=22.25

Likewise, the get_products() method, which follows the convention: GET localhost:5000/product/123, would return a JSON object of the specific product, including its price.

That said, we need not have special query_price(id) and get_price(id) methods.

@mansivirani , @rohithreddy1 , what are your thoughts?

mansivirani commented 7 years ago

Yeah I agree with @ephraimrosenfeld . I think the query of listing products list_products() can be given the min/max attributes.

NicholasHyland commented 7 years ago

I understand that the get_price(id) is not necessary anymore. It will follow from get_products() method.

But I thought that the list_products() method would literally list all the products, which is different to querying by price? There are also two issues for both.

Following the suggestion of John, I was going to add a min and max price attribute to my query_price() method: GET /products/{id}?min_price=100, max_price=200 - but this would be from my query_price() method?

NicholasHyland commented 7 years ago

You know what... I think I understand actually. Please confirm.

The list_products() method will list all the products UNLESS SPECIFIED BY PRICE?

That is, GET/products will list all products.

Whereas, GET/products?min_price=100, max_price=200 will list all products with that price range?

ephraimrosenfeld commented 7 years ago

Yes, list_products() will return all products, except those that are filtered out by whatever URL-attribute is added to the GET request.

I would suggest creating a helper method, e.g. matches_price(min, max, price), that can be called inside the list_products() method, similar to the matchesClause() method.

Come to think of it, I believe camel-case matchesClause() is more of a Java convention, whereas in Python we'd call it matches_clause(). It's aesthetic but once we are refactoring we might as well conform to the Python conventions.

ephraimrosenfeld commented 7 years ago

I would also add, that min and max can be independent of each other; having one without the other is valid, and would return those products above/below the price threshold.

That said, in the unit tests, you could have three products with prices: (Prod1 ->$10, Prod2 ->$15, Prodc->$20, respectively - with the following test cases:

  1. GET /product?min=11 -> returns Prod2 and Prod3
  2. GET /product?max=19 -> returns Prod1 and Prod2
  3. GET /product?min=14&max=16 -> returns Prod2

My $0.02.