-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 #1627
base: master
Are you sure you want to change the base?
Solution #1627
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! Your implementation of the Car
and CarWashStation
classes is mostly correct and meets many of the task requirements. However, there are a couple of critical issues that need to be addressed before we can approve the pull request:
-
Division by Zero: In the
calculate_washing_price
method, there's a potential division by zero error ifdistance_from_city_center
is zero. This could cause the program to crash, so it's important to add a check to ensure this value is not zero before performing the division. -
Car Washing Logic: The logic in the
serve_cars
method should ensure that a car is only washed if it is not already clean enough. Please double-check that this aligns with the intended behavior of the method.
Once these issues are resolved, your code will be in great shape! Keep up the excellent work, and don't hesitate to reach out if you have any questions or need further clarification. 😊
✨ 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.
washing_price = (car.comfort_class | ||
* (self.clean_power - car.clean_mark) | ||
* self.average_rating | ||
/ self.distance_from_city_center) |
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.
There is a potential division by zero error here if distance_from_city_center
is zero. Consider adding a check to ensure this value is not zero before performing the division.
for car in cars: | ||
|
||
# Obliczenie przychodu | ||
if car.clean_mark < self.clean_power: |
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 condition here checks if the car's clean_mark
is less than clean_power
, which is correct for determining if the car needs washing. However, ensure that this logic aligns with the intended behavior of the serve_cars
method, which should only wash cars that are not clean enough.
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.
Review Summary
Great progress! 🚗🧼
Your implementation of the Car
and CarWashStation
classes has improved significantly. You've successfully addressed the division by zero issue by ensuring that distance_from_city_center
is greater than zero before using it, which is crucial for preventing errors in the calculate_washing_price
method.
The logic in serve_cars
is now correctly aligned with the task requirements, ensuring that only cars with a clean_mark
less than the clean_power
are washed. This is a key improvement and ensures the functionality works as intended.
However, there's a minor point to consider: if distance_from_city_center
is zero, it should be set to a default value or raise an exception to handle this scenario properly. This will help prevent any potential issues in the future.
Overall, your code meets the task requirements, and the decision to approve is made to help you move forward with your learning. Please review the comments to further refine your code. Keep up the great work and continue to learn from these experiences! 🌟
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
if distance_from_city_center > 0: | ||
self.distance_from_city_center = distance_from_city_center |
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 check for distance_from_city_center
being greater than zero is good to prevent division by zero. However, if distance_from_city_center
is zero, it should be set to a default value or raise an exception to handle this case properly.
if car.clean_mark < self.clean_power: | ||
income += self.calculate_washing_price(car) | ||
|
||
self.wash_single_car(car) # Mycie samochodu |
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 logic here ensures that a car is only washed if its clean_mark
is less than clean_power
, which aligns with the requirement that a car should only be washed if it is not clean enough. This is correct.
No description provided.