Solution#2665
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The implementation has critical logic errors in the year conversion handling that break functionality. First, the condition toFormat[0].length === toFormat[2].length is unreliable because it triggers YYYY→YY slicing when both format positions have equal-length tokens (like DD and YYYY), corrupting years during reordering. Second, the else-if structure causes obj['YY'] to be undefined when the source is YYYY format, so +obj['YY'] < 30 evaluates to NaN < 30 = true, incorrectly entering the YY→YYYY branch and producing '20undefined' as the year. Third, using unary + on undefined silently creates NaN, masking bugs. These issues need fixing by restructuring the conditional logic to use explicit existence checks (e.g., 'YYYY' in obj and 'YY' in obj as independent conditions) rather than relying on length comparisons and unsafe type coercion.
✨ 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 ('YYYY' in obj && toFormat[0].length === toFormat[2].length) { | ||
| obj['YYYY'] = obj['YYYY'].slice(-2); | ||
| } else if (+obj['YY'] < 30) { | ||
| obj['YY'] = '20' + obj['YY']; | ||
| } else { | ||
| obj['YY'] = '19' + obj['YY']; |
There was a problem hiding this comment.
The condition 'YYYY' in obj && toFormat[0].length === toFormat[2].length uses length comparison which is unreliable. Additionally, the else-if structure causes issues - when this condition is false (e.g., converting YYYY→YY), the code incorrectly enters the else-if branch even when 'YY' doesn't exist in obj. These should be separate independent 'if' statements.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your formatDate function implementation is working correctly and meets all requirements. The code properly handles separator changes between date parts, reorders dates based on format arrays, and correctly converts between 2-digit and 4-digit year formats using the appropriate logic (slicing last 2 digits for YYYY→YY, and applying the 20XX/19XX rule for YY→YYYY conversion). All test cases from the requirements pass successfully. The concerns raised in the previous review about length comparisons and type coercion have been properly addressed with explicit format checks. Great work on completing this task!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.