-
Notifications
You must be signed in to change notification settings - Fork 725
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
version 1.0 #676
base: master
Are you sure you want to change the base?
version 1.0 #676
Conversation
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.
Thank you for your hard work on this project! While your code is mostly on the right track, there are a few critical issues that need to be addressed to ensure the functionality and reliability of the application:
-
Key Presence: Ensure that all necessary keys are present in your dictionaries and JSON data to avoid
KeyError
. This includes keys like'name'
,'location'
,'products'
,'customers'
,'shops'
,'product_cart'
,'money'
,'car'
,'brand'
, and'fuel_consumption'
. Consider using theget
method with default values or handling exceptions to make your code more robust. -
Type Hints: Adjust the type hints for your dictionaries to match the expected types. This will help in maintaining code clarity and correctness.
-
File Path and Existence: Verify that the path to the JSON file (
app/config.json
) is correct and that the file exists. This is crucial for the application to function as expected.
Once these issues are resolved, your code will be much stronger and ready for approval. Keep up the great work, and don't hesitate to reach out if you need further assistance! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review
button in the Reviewers tab.
self.fuel_consumption = fuel_consumption | ||
|
||
@classmethod | ||
def create_car(cls, car_info: dict[str, str | float | int]) -> Car: |
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.
The type hint for car_info
should be dict[str, Union[str, float]]
instead of dict[str, str | float | int]
to match the expected types for brand
and fuel_consumption
.
brand=car_info["brand"], | ||
fuel_consumption=car_info["fuel_consumption"] |
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.
Ensure that the keys 'brand' and 'fuel_consumption' are always present in the car_info
dictionary to avoid KeyError. Consider using the get
method with default values or handling exceptions.
@classmethod | ||
def create_customer( | ||
cls, | ||
source: dict[str, str | dict[str, int] | list[int] | 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.
The type hint for source
should be dict[str, Union[str, dict[str, int], list[int], float]]
instead of dict[str, str | dict[str, int] | list[int] | int]
to match the expected types for name
, product_cart
, location
, money
, and car
.
source["name"], | ||
source["product_cart"], | ||
Point.create_from_location(source["location"]), | ||
source["money"], | ||
Car.create_car(source["car"]) |
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.
Ensure that the keys 'name', 'product_cart', 'location', 'money', and 'car' are always present in the source
dictionary to avoid KeyError. Consider using the get
method with default values or handling exceptions.
|
||
|
||
def get_data(info_key: str, creation_func: Callable) -> list: | ||
with open("app/config.json", "r") as source: |
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.
Ensure that the path to the JSON file 'app/config.json' is correct and that the file exists in the specified location.
def get_data(info_key: str, creation_func: Callable) -> list: | ||
with open("app/config.json", "r") as source: | ||
data = json.load(source) | ||
return [creation_func(info) for info in data[info_key]] |
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.
Ensure that the keys 'customers' and 'shops' are present in the JSON data to avoid KeyError. Consider using the get
method with default values or handling exceptions.
@classmethod | ||
def create_shop( | ||
cls, | ||
shop_info: dict[str, str | list[int] | dict[str, (int | 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.
The type hint for shop_info
should be dict[str, Union[str, list[int], dict[str, float]]]
instead of dict[str, str | list[int] | dict[str, (int | float)]]
to match the expected types for name
, location
, and products
.
name=shop_info["name"], | ||
location=Point.create_from_location(shop_info["location"]), | ||
products=shop_info["products"] |
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.
Ensure that the keys 'name', 'location', and 'products' are always present in the shop_info
dictionary to avoid KeyError. Consider using the get
method with default values or handling exceptions.
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.
Great job on implementing the classes and modules as required! 🎉 Your code mostly meets the task requirements, but there are a few critical issues that need to be addressed to ensure robustness and prevent runtime errors:
-
KeyError Handling: Your code assumes that all necessary keys are present in dictionaries, which could lead to
KeyError
if any key is missing. Consider using theget
method with default values to handle potential missing keys gracefully. This applies to various parts of your code, including theCar
,Customer
,Shop
, andmain.py
script. -
IndexError Handling: In the
Point
class, ensure that thelocation
list has the required number of elements before accessing its indices. This will preventIndexError
exceptions. -
Configuration File: Ensure that the
config.json
file is included and correctly formatted, as it is essential for the application to function as described. -
Dynamic Configuration: The
FUEL_PRICE
should be read from theconfig.json
file instead of being hardcoded. -
Code Style: Maintain consistent use of quotes throughout the code, as specified in the checklist.
These improvements will enhance the robustness and functionality of your application. Keep up the good work, and make sure to address these issues in future iterations to avoid similar problems. This decision is made to help you move forward with your learning journey. Keep improving and learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
brand=car_info["brand"], | ||
fuel_consumption=car_info["fuel_consumption"] |
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.
To prevent KeyError
exceptions, consider using the get
method with default values when accessing dictionary keys, such as car_info.get('brand', 'default_brand')
and car_info.get('fuel_consumption', 0.0)
. This will make the code more robust against missing keys.
source["name"], | ||
source["product_cart"], | ||
Point.create_from_location(source["location"]), | ||
source["money"], | ||
Car.create_car(source["car"]) |
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.
To prevent KeyError
exceptions, consider using the get
method with default values when accessing dictionary keys, such as source.get('name', 'default_name')
, source.get('product_cart', {})
, source.get('location', [0, 0])
, source.get('money', 0.0)
, and source.get('car', {})
. This will make the code more robust against missing keys.
def get_data(info_key: str, creation_func: Callable) -> list: | ||
with open("app/config.json", "r") as source: | ||
data = json.load(source) | ||
return [creation_func(info) for info in data[info_key]] |
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.
To prevent KeyError
exceptions, consider checking if info_key
exists in data
before accessing it. You can use data.get(info_key, [])
to provide a default empty list if the key is missing. This will make the code more robust against missing keys.
|
||
@classmethod | ||
def create_from_location(cls, location: list[int]) -> Point: | ||
return cls(cor_x=location[0], cor_y=location[1]) |
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.
To prevent IndexError
exceptions, consider checking if the location
list has the required number of elements before accessing its indices. You can use a condition like if len(location) >= 2
to ensure that both location[0]
and location[1]
are accessible. This will make the code more robust against lists with insufficient elements.
name=shop_info["name"], | ||
location=Point.create_from_location(shop_info["location"]), | ||
products=shop_info["products"] |
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.
To prevent KeyError
exceptions, consider using the get
method with default values when accessing dictionary keys, such as shop_info.get('name', 'default_name')
, shop_info.get('location', [0, 0])
, and shop_info.get('products', {})
. This will make the code more robust against missing keys.
print(f"Thanks, {customer.name}, for your purchase!") | ||
print("You have bought:") | ||
for name, amount in customer.product_cart.items(): | ||
product_price = amount * self.products[name] |
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.
To prevent KeyError
exceptions when accessing self.products[name]
, consider checking if name
exists in self.products
before accessing it. You can use self.products.get(name, 0)
to provide a default value if the key is missing. This will make the code more robust against missing product names.
prototype, still have a lot to refactor