solution#2671
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The code has a functional bug in the YY→YYYY conversion for edge case '00'. When the value is 0, it gets converted to string '00' before the comparison, causing '00' < 30 to perform lexicographic string comparison instead of numeric comparison. This results in '00' being incorrectly converted to 1900 instead of 2000. To fix this, ensure numeric comparison by using parseInt(value, 10) directly in the ternary condition on line 34, or remove lines 30-32 which unnecessarily convert to string. The general review summary incorrectly dismissed this as a non-issue by claiming JavaScript coerces strings to numbers in comparisons—however, the coercion only happens when comparing a string to a number, not when both operands are strings, which is what occurs here after the explicit string conversion.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
|
|
||
| if (yy === 0) { | ||
| yy = '00'; | ||
| } | ||
|
|
There was a problem hiding this comment.
When yy is 0, it's converted to string '00'. Then '00' < 30 uses string comparison (which is false), causing 2000-02-18 to become 1900 instead of 2000. Either remove lines 30-32 or ensure numeric comparison by using the number 0 directly in the ternary on line 34.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution correctly implements all requirements for the formatDate function. All functional requirements pass: separator changes, date part reordering, and year format conversions (YYYY↔YY) work as specified. The previous review's concern about a '00' year edge case was unfounded—JavaScript's type coercion handles this correctly. There's a minor code quality suggestion: the conversion logic around lines 30-32 is redundant. Since parseInt already converts yy to a number, you can simplify by removing those lines and keeping yy as a numeric type throughout the comparison. This makes the code clearer without changing behavior. The core functionality is solid and all tests pass. Great work on meeting all the requirements!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.