Store interface

Question

I've written classes Store, Cart, User that manages methods that allow the user to add/remove products from a shopping cart, as well as being able to query different descriptions about the item (id, price, quantity). The User class hasn't fully been utilized yet. I'm wondering if there is any way to make this more compact/efficient? I use some recursion while generating an ID, but I'm wondering if it can be implemented in a better way. Any and all suggestions are welcomed! Thank you! There is also a test file (linked at bottom) I use to generate items and test the methods within the class.

store.py

import random
class Store():
 def __init__(self):
 self.products = {}
 self.product_ids = []
 """
 Adds a product dict to the self.products dict
 """
 def add_product(self, product):
 self.products[product['name']] = {
 'id': self.generate_product_id(),
 'price': product['price'],
 'quantity': product['quantity']
 }
 """
 Removes a product dict from self.products dict, unless the user is not
 an admin or the store doesn't have the item, then it raises an exception
 """
 def delete_product(self, product, user):
 if not user.is_admin():
 raise Exception('Must be admin to delete')
 if not self.has_product(product_id):
 raise Exception('Unknown product id')
 del self.products[product['name']]
 """
 Returns true if the product id passed does not appear in the
 product_ids array, else returns false
 """
 def existing_id(pid):
 for x in self.product_ids:
 if pid == x:
 return True
 return False
 """
 Returns a random number used to distinguish between products
 and makes sure it isn't already in use by appending it to an
 array.
 """
 def generate_product_id(self):
 rnum = random.randint(100000,999999)
 if not existing_id(rnum):
 self.product_ids.append(rnum)
 return rnum
 else:
 self.generate_product_id()
 """
 Methods for getting product information
 """ 
 def get_product(self, product):
 return self.products[product['name']]
 def get_product_id(self, product):
 return self.products[product['name']]['id']
 def get_product_price(self, product):
 return self.products[product['name']]['price']
 def get_product_quantity(self, product):
 return self.products[product['name']]['quantity']
 """
 Reduces quantity of specified product, unless amount is greater than
 the product['quantity'], then it raises an exception
 """
 def reduce_quantity(self, product, amount):
 if amount > self.products[product['name']]['quantity']:
 raise Exception('Amount greater than quantity!')
 self.products[product['name']]['quantity'] -= amount

cart.py

class Cart():
 def __init__(self):
 self.items = {}
 self.final_price = 0
 """
 Adds product to cart, then updates final price
 """
 def add_to_cart(self, product):
 self.items[product['name']] = {
 'price': product['price'],
 'quantity': product['quantity']
 }
 self.final_price += (product['price'] * product['quantity'])
 """
 Prints each element of self.items, their price & quantity
 """
 def print_cart(self):
 for x in self.items:
 print(x)
 print(' ' + "Price: $" + str(self.items[x]['price']))
 print(' ' + "Quantity: " + str(self.items[x]['quantity']))
 """
 Returns self.final_price, being calculated in 'add_to_cart'
 """
 def get_final_price(self):
 return self.final_price

user.py

class User():
 def __init__(self, admin=False):
 self.admin = admin
 def is_admin(self):
 return self.admin

main.py

from store import *
from cart import *
from user import *
def main():
 store = Store()
 cart = Cart()
 name = ['Fancy Shoes', 'Fancy Pants']
 price = [147.00, 162.00]
 quantity = [10, 8]
 for i in range(len(name)):
 product = {
 'name': name[i],
 'price': price[i],
 'quantity': quantity[i]
 }
 store.add_product(product)
 cart.add_to_cart(product)
 cart.print_cart()
 print("Final Price: $" + str(cart.get_final_price()))
if __name__ == '__main__':
 main()

Answer 1

1. For efficiency, I would change Store to have only a products attribute, which maps the product id to the product object. You can then have functions which look up products by name, price, etc. if desired.

2. Bug:

   def generate_product_id(self):
    rnum = random.randint(100000,999999)
    if not existing_id(rnum):
    self.product_ids.append(rnum)
    return rnum
    else:
    # FIX
    return self.generate_product_id()
   

3. Some of your methods are unnecessary:

   def get_product_price(self, product):
    return self.products[product['name']]['price']
   

   Simply use product['price'] instead of calling this method with a product.

4. Perhaps it makes sense to also have a Product class in the future.

5. Cart.items makes more sense as a list I think.

6. Cart.final_price: I would calculate it dynamically, which makes code for adding and removing products simpler. Performance difference is insignificant.

7. Finally, in Python, the docstring is placed inside the function (doesn't change the output, but allows Python documentation tools to recognize your docstrings):

   def my_function():
    """ Docstring """
    return True
   

Answer 2

I agree with all points mentioned in the other answer by @pj.dewitte.

In addition, in Python it is frowned upon to write explicit getters and setters for your attributes if they are not needed. Just use the attribute directly. If you need to add more stuff around it later, then you can still write a getter/setter and use @property so the interface does not change.

class User():
 def __init__(self, admin=False):
 self.is_admin = admin

Python also has an official style-guide, PEP8. It recommends using a consistent amount of whitespace for indentation. Currently you seem to be using two spaces for methods and four spaces for indentation within those methods. Just use four spaces everywhere.

Python has something called magic methods (sometimes also called dunder methods, for "double-underscore", you will see why in a moment). They allow you to give your custom classes built-in behavior. For example, if you implement the __str__ method, you can then do print(obj) and this method will be called internally to create a readable string to display to the user.

In Python 3.6, f-strings were introduced (described in PEP498), which makes writing formatted strings a lot easier.

class Cart():
 def __init__(self):
 self.items = {}
 def __str__(self):
 """
 Returns a string with each element of self.items, their price & quantity
 """
 lines = []
 for name, product in self.items.items():
 lines.append(name)
 lines.append(f" Price: ${product['price']}")
 lines.append(f" Quantity: {product['quantity']}")
 return "\n".join(lines)
 @property
 def final_price(self):
 return sum(product["price"] * product["quantity"]
 for product in self.items.values())
 def add_to_cart(self, product):
 """Adds product to cart."""
 self.items[product['name']] = {
 'price': product['price'],
 'quantity': product['quantity']
 }

By making final_price a property that is evaluated everytime it is accessed, this also allows you to implement delete_item_from_cart and reduce_quantity_in_cart methods without having to make sure to always update the price properly. Of course if your carts become very large (> 10000 items), doing it like you did with an attribute that is updated where necessary will be faster.

One other point: It might make sense to make cart.items into a list, instead of a dict. Or at least add additional checks to add_to_cart against an item being added multiple times (with a dict it will just be overwritten, instead of the quantity added and checked that it actually is the same product and does not just have the same name).

• pj.dewitte: This actually clarifies some points in my answer, good work!