Develop #4872
Develop #4872
Conversation
There was a problem hiding this comment.
Thank you for your effort on this task! 😊 While your code is well-structured and meets many of the task requirements, there's a critical issue with the discount logic that needs to be addressed. The function should apply the correct discount based on the number of rental days. Specifically, if the rental period qualifies for both short-term and long-term discounts, only the long-term discount should be applied. Please adjust the logic to ensure that the appropriate discount is applied based on the number of days. Once this is fixed, your solution will be ready for approval! 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 >= longTerm) { | ||
| return rentalPrice - longTermDiscount; | ||
| } | ||
|
|
||
| if (days >= shortTerm) { | ||
| return rentalPrice - shortTermDiscount; |
There was a problem hiding this comment.
The logic for applying discounts is incorrect. If the number of days is greater than or equal to the long-term threshold, the long-term discount should be applied. However, if the number of days is only greater than or equal to the short-term threshold but less than the long-term threshold, the short-term discount should be applied. The current implementation will not apply the short-term discount if the days are greater than or equal to the long-term threshold, which is correct, but the logic should ensure that only the appropriate discount is applied.
There was a problem hiding this comment.
Review Summary
Great job on implementing the calculateRentalCost function! 🎉 Your code meets the task requirements and constraints, with well-chosen variable names and a clean structure. Here are a few points to consider for improvement:
-
Discount Logic: The current logic applies either a short-term or a long-term discount, but not both. If the intention is to apply both discounts cumulatively for rentals longer than 7 days, you'll need to adjust the logic accordingly.
-
Code Readability: You've done well by using
constfor variables and avoiding hardcoding values directly in expressions. This makes your code more maintainable and easier to understand. -
Simplified Conditional Logic: Your use of
ifstatements withreturnis efficient and keeps the code clean.
Remember, addressing the discount logic issue will ensure the function behaves as expected in all scenarios. Keep up the good work, and continue to refine your skills! If you have any questions or need further clarification, feel free to reach out. Happy coding! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/calculateRentalCost.js
Outdated
| if (days >= longTerm) { | ||
| return rentalPrice - longTermDiscount; | ||
| } else if (days >= shortTerm) { | ||
| return rentalPrice - shortTermDiscount; |
There was a problem hiding this comment.
The current logic applies a long-term discount for rentals of 7 days or more and a short-term discount for rentals of 3 to 6 days. However, the discount is applied only once, and the function returns immediately after applying the first applicable discount. This logic is correct if only one discount is meant to be applied. If the intention is to apply both discounts cumulatively for rentals longer than 7 days, the logic needs to be adjusted.
src/calculateRentalCost.js
Outdated
| @@ -4,7 +4,22 @@ | |||
| * @return {number} | |||
| */ | |||
| function calculateRentalCost(days) { | |||
| // write code here | |||
| const shortTerm = 3; | |||
There was a problem hiding this comment.
| const shortTerm = 3; | |
| const SHORT_TERM = 3; |
Consider using the checklist https://github.com/mForkosh/js_task-transportation-on-vacation/blob/develop/checklist.md to complete the task
No description provided.