-
Notifications
You must be signed in to change notification settings - Fork 696
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
Solution Version 1.00 #608
base: master
Are you sure you want to change the base?
Conversation
app/customer.py
Outdated
print(f"{self.name}'s trip to the {shop.name} costs {cost_of_trip}") | ||
self.shop_trips[cost_of_trip] = shop | ||
|
||
def get_distance_price(self, shop_location: dict) -> float: |
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.
this would be rather a Car method - not a customer method. it doesn't depend on a customer in any way so it would be logical to move it out. Your customer already has a lot of methods and having one class that does it all is not always a good idea
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.
I also had doubts about this method. Now it’s clear :) Thanks!
app/main.py
Outdated
] | ||
shops = [Shop.get_shop_from_dict(shop) for shop in shops_dict] | ||
|
||
for customer in customers: |
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.
Three for loops over the same data is a bit too much. You can reduce it to two loops for efficeincy. I would recommend to create Shops first, and then iterate over same data set to create customers, cars and do other things required
app/customer.py
Outdated
self, | ||
name: str, | ||
product_cart: dict, | ||
location: dict, |
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.
I think you could implement a class Location that will have coords as parameters on init and will have calculate_distance method. This will structure your code better
app/shop.py
Outdated
self, | ||
name: str, | ||
location: dict, | ||
products: dict |
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 could also be presented as separate class and have method calcuclate_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.
Looks good 🌸
No description provided.