Skip to content
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 #1759

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Solution #1759

wants to merge 7 commits into from

Conversation

mykyta-mitin
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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 the task requirements. However, there are a couple of critical issues that need to be addressed before we can approve the pull request:

  1. Division by Zero: In the calculate_washing_price method, there's a potential division by zero error when using distance_from_city_center. Please add a check to ensure this value is not zero before performing the division to prevent runtime errors.

  2. Empty List Check: In the serve_cars method, it's important to check if the list of cars is not empty before proceeding with calculations. This will help avoid unnecessary computations and ensure the robustness of your code.

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 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.

app/main.py Outdated
price = (
car.comfort_class *
(self.clean_power - car.clean_mark) *
(self.average_rating / self.distance_from_city_center)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential division by zero error: Ensure that distance_from_city_center is not zero before performing this division to avoid runtime errors.

if car.clean_mark < self.clean_power:
car.clean_mark = self.clean_power

def serve_cars(self, cars: list[Car]) -> float:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider checking if the cars list is not empty before proceeding with the loop to avoid unnecessary calculations.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

🎉 Congratulations on your progress! Your implementation of the Car and CarWashStation classes has been significantly improved. You've successfully addressed the division by zero issue by adding a check in the calculate_washing_price method, and you've ensured robustness by checking for an empty list in the serve_cars method. These changes make your code more reliable and prevent potential runtime errors.

Your code now meets the task requirements and constraints, and it follows the guidelines provided. While there were issues initially, you've resolved them effectively.

Please take note of the comments provided in the review to further refine your skills. Remember, addressing critical issues like these in future projects will enhance the quality of your code. Keep up the great work and continue learning from each experience! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants