Skip to content

solution#2673

Open
oleshna27l-debug wants to merge 2 commits into
mate-academy:masterfrom
oleshna27l-debug:develop
Open

solution#2673
oleshna27l-debug wants to merge 2 commits into
mate-academy:masterfrom
oleshna27l-debug:develop

Conversation

@oleshna27l-debug

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 current formatDate implementation still breaks core functionality required by the task. Specifically, you treat the separator in both fromFormat and toFormat as if it were a date part, which leads to undefined values in the mapping object and extra separators in the final output (for example, outputs like 2020.02.18. or 2020-02-18-). This violates the requirement that fromFormat[3] and toFormat[3] be used as separators only and that the result have no trailing separator. To fix this, you should: (1) split the input date using fromFormat[3] as the separator; (2) map only indices 0–2 of fromFormat to result (ignoring the separator element); and (3) when building the output, only push parts for indices 0–2 of toFormat into output and then join with toFormat[3]. These changes will also resolve the unit test failures where year mapping is correct in concept but polluted by the misuse of the separator. A smaller improvement (not a blocker) would be to normalize year keys once (e.g., always storing as YYYY, MM, DD) to simplify your mapping logic. You already have the right idea for YYYYYY conversion, so once the separator and indexing issues are corrected, your solution should fully meet the requirements.


✨ 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

const result = {};

for (let i = 0; i < fromFormat.length; i++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

fromFormat includes both the three date parts and a separator as the 4th element, but this loop treats every entry as a part; you should only map the actual date parts (e.g. indices 0–2) and ignore the separator element.

Comment thread src/formatDate.js Outdated

const output = [];

for (let i = 0; i < toFormat.length; i++) {

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 you push result[toFormat[i]] for every element in toFormat, including the separator at index 3, which leads to undefined entries and extra separators in the final string; restrict this loop to the date parts only and keep using toFormat[3] for join.

@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 behaves correctly for all specified examples. The function now cleanly separates the concept of date parts from the separator, using fromFormat[3] only to split the input string and toFormat[3] only to join the output, which matches the task’s expectations. You correctly handle reordering of DD, MM, and YYYY/YY, and your year conversion logic (YYYY → YY via the last two digits and YY → YYYY with the <30 → 20YY / >=30 → 19YY rule) exactly follows the given specification. There is no unnecessary input validation, which aligns with the explicit instruction to assume well-formed inputs.

As a minor, non-blocking improvement, you could consider normalizing the year representation internally (e.g., always storing years as YYYY and only shortening to YY at output) to simplify condition checks, but this is not required for correctness. Overall, the implementation is clear, correct, and aligns well with the project requirements—nice work on addressing the earlier issues so precisely.


✨ 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