-
Notifications
You must be signed in to change notification settings - Fork 0
implemented conv store task #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
shorodilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Треба зробити рефакторінг та додати відсутні методи.
01/task_01.py
Outdated
|
|
||
| def get_total(self, count: float): | ||
| """Calculating total price for the product""" | ||
| return self.price * count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут треба полагодити, бо можлива неочікувана поведінка.
Наприклад
product = Product("product", 15.10)
product.get_total(0.7)
01/task_01.py
Outdated
| """Adding products and calculating total sum""" | ||
|
|
||
| self.products[product] += count | ||
| self.total += product.price * count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Он у тебе више є метод для цього.
01/task_01.py
Outdated
| """Describing shopping cart""" | ||
|
|
||
| def __init__(self): | ||
| self.products = defaultdict(int) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не складай кастомні об'єкти без __hash__ до ключей словника.
Використовуй список.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Або роби реалізацію __hash__, проте списки ліпше.
|
Округлив суму. Використав список замість словника. Додав метод для підрахунку суми замість атрибута. |
shorodilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Треба зберігати інформацію про кількість товарів у кошику.
01/task_01.py
Outdated
| """Adding products to the cart""" | ||
|
|
||
| total_product_price = product.get_total(count) | ||
| self.total_cart.append(total_product_price) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
За такої реалізації легше було зробити одне поле, куди складати суму покупки.
Але таким чином проігноровано умову, зберігати кільсть товару у кошику.
Це треба доробити.
|
Додав ще розрахунок загальної к-ті товарів в кошику, правильно? Чи треба назву кожного товару та його кількість? |
Так, окремо товар та його кількість. |
|
Змінив на словник products_list, де міститься товар, його ціна, к-сть та сума, як в чеку. Загальна суму тепер береться з цього словника |
Ок, а як мені дістатись до об'екту товару? |
shorodilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Губиш об'єкти товарів при додавані до кошика.
01/task_01.py
Outdated
| """Adding products to the cart""" | ||
|
|
||
| if product.name in self.products_list: | ||
| quantity = self.products_list.get(product.name, 0)["quantity"] + count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Це важко читати. У такому випадку ліпше розбити на декілька рядків.
01/task_01.py
Outdated
| quantity = self.products_list.get(product.name, 0)["quantity"] + count | ||
| else: | ||
| quantity = count | ||
| product_sum = product.get_total(quantity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
От цьому тут явно не місце
01/task_01.py
Outdated
| quantity = count | ||
| product_sum = product.get_total(quantity) | ||
| price = product.price | ||
| self.products_list[product.name] = {"quantity": quantity, "price": price, "product_sum": product_sum} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У тебе губиться об'ект, тобто якщо знадобиться ще якийсь метод класу Product- то це буде неможливо.
|
Додав об'єкт замість атрибутів продукта. Переніс підрахунок суми в метод суми. Правильно зрозумів? |
shorodilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Метод ShoppingCart.get_total потребує рефакторінгу.
01/task_01.py
Outdated
| for item in self.products_list: | ||
| p_price = self.products_list[item]['product'].price | ||
| quantity = self.products_list[item]["quantity"] | ||
| product_list_sum += p_price * quantity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Використовуй для цього Product.get_total
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
І про округлення не забувай.
product_1 = Product("product #1", 10.59)
product_2 = Product("product #2", 36.55)
cart = ShoppingCart()
cart.add_product(product_1, 0.7)
cart.add_product(product_2, 4)
cart.get_total()|
Переробив з використанням get_total та перевірив ще раз на округлення |
shorodilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Треба виправити зберігання товарів у кошику - тип ключа або власне тип змінної контейнера.
Метод для підрахунку суми товарів у кошику потребує рефакторінгу.
01/task_01.py
Outdated
| """Describing the shopping cart""" | ||
|
|
||
| def __init__(self): | ||
| self.products_list = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Погана ідея використовувати словник з кастомним класом без __eq__ та __hash__ у якості ключа.
Розвали цю штуку на два списки - за швидкістю не втратиш, але спростиш реалізацію.
Якщо дуже бажаєш мати словник, то ключем я б радив взяти product.name (str) або (product.name, product.price) (tuple).
01/task_01.py
Outdated
| product = self.products_list[item]['product'] | ||
| quantity = self.products_list[item]["quantity"] | ||
| product_list_sum += product.get_total(quantity) | ||
| return product_list_sum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
От тут буде проблема
product_1 = Product("foo", 10.59)
product_2 = Product("bar", 36.55)
cart = ShoppingCart()
cart.add_product(product_1, 0.7)
cart.add_product(product_2, 4)
cart.total_cart_sum()
01/task_01.py
Outdated
| """Calculating the total cart sum""" | ||
|
|
||
| product_list_sum = 0 | ||
| for item in self.products_list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Бачиш, не зручно ітеруватись за словником, бо у тебе доволі важкі структуру даних.
Якщо розбити на списки - буде набагато легше.
|
Розбив на 2 списки - окремо по екземплярам класу Product, та окремо кількості. Суму подраховую беручи дані з цих списків. Також додав округлення до суми, хоча так і не зрозумів, чому до кожного окремого об'єкту метод get_total округляє, а в сумі чомусь не округляє. |
shorodilov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
01/task_01.py
Outdated
| product.name, 0 | ||
| )["quantity"] + count | ||
| if product in self.products_list: | ||
| self.products_count[self.products_list.index(product)] += count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Так норм, але важко читати - краще розбити на два рядки.
01/task_01.py
Outdated
| cart1.add(prod1, 5) | ||
| print(cart1.products_list) | ||
| print(cart1.total_cart_sum()) | ||
| for i in range(len(self.products_list)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| for i in range(len(self.products_list)): | |
| for product, count in zip(self.products_list, self.products_count): |
|
Поправив, дякую |
No description provided.