Skip to content

Add solution for formatDate.js#2687

Open
JuliaTr wants to merge 1 commit into
mate-academy:masterfrom
JuliaTr:develop
Open

Add solution for formatDate.js#2687
JuliaTr wants to merge 1 commit into
mate-academy:masterfrom
JuliaTr:develop

Conversation

@JuliaTr

@JuliaTr JuliaTr commented Jul 8, 2026

Copy link
Copy Markdown

Solved problem converting date from one format to another one.

@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 there are still critical logic issues in src/formatDate.js that break the required functionality. The main blocker is the year conversion: in the inner loop, when converting between YYYY and YY, your code pushes both the converted year and the original year segment, so cases like YYYY → YY can produce outputs such as 20.02.2020 instead of 20.02.20 (see the branches around the if (currentFormat === 'YY' && value === 'YYYY') and the generic if (value === currentFormat) that follows). Similarly, the YY → YYYY path first appends the converted 4‑digit year and then also appends the 2‑digit year, which violates the requirement that the output must contain only the target format representation.

A second critical issue is the mapping between date segments and format tokens in createObjectOriginalFormat (around the lines where matchOriginalFormat is built). You currently use the date part string (e.g. '2020') as the object key and the format token (e.g. 'YYYY') as the value, then later iterate with Object.entries and treat the key as the date part and the value as its format. Because objects are keyed by the segment values rather than their positions, ordering is lost and the association between fromFormat and toFormat becomes unreliable, which can lead to incorrect reordering and transformations. To fix this, work positionally: for each index i, dateSplit[i] should correspond to fromFormat[i], and for each toFormat token you should select the correct source segment based on its index or a well-defined mapping from tokens to indices.

Concretely, you need to (1) make the year-conversion branches mutually exclusive with the generic if (value === currentFormat) fallback (e.g. by using else if or continue after a conversion) so only one representation is pushed per segment, and (2) refactor the mapping logic to use arrays or a consistent token→index mapping instead of an object keyed by date values. These two fixes are required for your function to reliably meet the task requirements of changing separators, reordering parts, and converting between YY and YYYY for the documented examples. Once you address these issues, your overall structure should work well, and the solution will be much more robust. Keep going—you’re close; the main work left is tightening this mapping and conversion logic.


✨ 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
let idx = 0;

for (const number of dateSplit) {
matchOriginalFormat[number] = fromFormat[idx];

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 build matchOriginalFormat as { [datePart]: formatToken }. This reverses the natural mapping and makes later logic treat key as the date value and value as its format, but because keys are the date strings, ordering is lost and matching by format becomes error-prone. Consider mapping format tokens to date parts by index instead of by value.

Comment thread src/formatDate.js
Comment on lines +39 to +40
if (currentFormat === 'YY' && value === 'YYYY') {
newFormat.push(key.slice(2));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When converting from YYYY to YY, this branch correctly slices the last two digits, but the generic if (value === currentFormat) below will also run for the same entry, causing both YY and YYYY forms to be pushed. You need to prevent the fallback push from executing when a conversion case has already been handled.

Comment thread src/formatDate.js
Comment on lines +43 to +49
if (value === 'YY' && currentFormat === 'YYYY') {
if (Number(key) < MAX) {
newFormat.push(`20${key}`);
}

if (Number(key) >= MAX) {
newFormat.push(`19${key}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similarly, when converting YY to YYYY, you push a 4-digit year in this block and then if (value === currentFormat) will also push the original 2-digit year, producing incorrect output. Make these condition blocks mutually exclusive (e.g. use else if or early continue) so only the required representation is appended.

Comment thread src/formatDate.js
Comment on lines +53 to +54
if (value === currentFormat) {
newFormat.push(key);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This fallback if (value === currentFormat) runs even after a conversion branch has already pushed a transformed value, leading to duplicate or wrong year values in the output. Restrict this branch to cases where no conversion (YY↔YYYY) applies.

@2pasha 2pasha 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.

lgtm 🔥

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.

3 participants