-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Solution #5081
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?
Solution #5081
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 the calculateRentalCost
function! 😊 While your implementation is mostly on the right track, there are a few critical areas that need to be addressed before we can approve the changes:
-
Use of Literals in Expressions: Your code currently uses hardcoded literals like
7
,50
,3
, and20
. It's important to replace these with descriptive constants to enhance readability and maintainability. This change will make your code easier to understand and modify in the future. -
Code Readability: Consider adding comments to explain the discount logic for rentals of 3 or more days and 7 or more days. This will help others (and your future self) to quickly grasp the purpose and functionality of your code.
These improvements are crucial for meeting the task requirements and ensuring that your code is clean and maintainable. Once you've made these changes, feel free to resubmit, and I'll be happy to take another look. Keep up the great work! 🚀
✨ 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.
src/calculateRentalCost.js
Outdated
if (days >= 7) { | ||
return basePrice - 50; | ||
} |
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 for applying a discount for rentals of 7 or more days is correct. However, consider adding comments to explain the discount logic for better code readability.
src/calculateRentalCost.js
Outdated
if (days >= 3) { | ||
return basePrice - 20; | ||
} |
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 for applying a discount for rentals of 3 or more days is correct. Similar to the previous block, adding comments to explain the discount logic would improve code readability.
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 job on improving the calculateRentalCost
function! 🎉 Your use of descriptive constants like DAILY_RATE
, LONG_TERM
, LONG_TERM_DISCOUNT
, MID_TERM
, and MID_TERM_DISCOUNT
enhances the readability and maintainability of the code. You've also successfully avoided hardcoded literals and used const
for variable declarations, which is excellent for preventing unintentional changes.
However, to further improve your code, consider adding comments to explain the discount logic, especially for rentals of 3 to 6 days and 7 or more days. This will help future readers understand the logic more easily.
Remember, the decision to approve is to help you move forward with your learning journey. Please review the comments to enhance your code. Addressing these suggestions will be beneficial for your future projects. Keep up the great work and continue to learn from your experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
if (days >= LONG_TERM) { | ||
return basePrice - LONG_TERM_DISCOUNT; |
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.
Consider adding a comment here to explain that a discount is applied for rentals of 7 or more days. This will help clarify the logic for future readers.
if (days >= MID_TERM) { | ||
return basePrice - MID_TERM_DISCOUNT; |
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.
Consider adding a comment here to explain that a discount is applied for rentals of 3 to 6 days. This will help clarify the logic for future readers.
No description provided.