Skip to content

Implement formatDate#2692

Open
DYM-d wants to merge 2 commits into
mate-academy:masterfrom
DYM-d:develop
Open

Implement formatDate#2692
DYM-d wants to merge 2 commits into
mate-academy:masterfrom
DYM-d:develop

Conversation

@DYM-d

@DYM-d DYM-d commented Jul 13, 2026

Copy link
Copy Markdown

No description provided.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because the YYYYYY conversion is currently incorrect and breaks a core requirement of the task. In src/formatDate.js, the logic that converts a 4‑digit year to 2 digits subtracts 1900 from the numeric year instead of taking the last two characters of the year string; this only works accidentally for some years and will produce wrong results for years outside the 1900s. The requirement explicitly states that when going from YYYY to YY, you should "use 2 last digits", so the implementation needs to be updated to derive the 2‑digit year via string slicing (for example, yearString.slice(-2)) rather than arithmetic. This is a critical blocker because it affects the core functionality of the formatter and will cause incorrect output for valid inputs that include years not equal to 19xx.

To fix this, locate the branch in your formatDate function where fromFormat uses YYYY and toFormat uses YY (the YYYYYY case) and change the year conversion so it takes the last two characters from the year string, e.g. const shortYear = fullYearString.slice(-2);, and then uses that value as the YY part. Make sure this logic works for any 4‑digit year, including 18xx, 19xx, 20xx and beyond, and does not attempt to infer or change the century during truncation. The rest of your implementation — changing separators, reordering parts, and handling YYYYYY with the 20/19 rule — looks solid and aligns well with the task description. Once you correct the YYYYYY handling, your solution will fully meet the requirements, and you’re very close to that point already.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/formatDate.js Outdated
newDate[toYearIndex] = century19 + +dateSplit[fromYearIndex];
}
} else {
newDate[toYearIndex] = dateSplit[fromYearIndex] - century19;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here, when converting from YYYY to YY, you subtract 1900 from the stringified year, which only works accidentally for some years and does not match the requirement "use 2 last digits"; instead, take the last two characters of the year string (e.g. via slice(-2)) so any 4-digit year is converted correctly to YY.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it fully meets the functional requirements for formatDate and matches all checklist items. The function correctly accepts date, fromFormat, and toFormat, uses the separator from fromFormat to split the input string, and the separator from toFormat to join the output, ensuring formats like DD/MM/YY and YYYY-MM-DD are handled as described. You properly reorder the date parts based on the positions of year, month, and day in the format arrays, which satisfies the requirement to support flexible format transformations. Year conversion logic is correctly implemented: YYYY → YY uses slice(-2) on the year string, and YY → YYYY uses 20YY for years under 30 and 19YY otherwise, aligning with [CHECKLIST ITEM #9#11]. The implementation also follows the constraint of assuming valid input and not adding extra defensive checks, matching [CHECKLIST ITEM #6#8]. Overall, the function behaves as required and there are no critical blockers, so your work on this task is successfully completed—nice job keeping the logic clean and focused on the specified behavior.


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

Footnotes

  1. Rate AI review example

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