Skip to content

First task solution#2682

Open
jkumorek01 wants to merge 3 commits into
mate-academy:masterfrom
jkumorek01:develop
Open

First task solution#2682
jkumorek01 wants to merge 3 commits into
mate-academy:masterfrom
jkumorek01:develop

Conversation

@jkumorek01

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 there is a critical, blocking issue in your implementation that prevents the function from working as required. In src/formatDate.js, the call date.split(baseElement) uses baseElement without ever defining it (ESLint no-undef and a runtime ReferenceError), so the code cannot run. Per the task, the input separator must come from the fromFormat array, with its last element representing the separator; you should derive something like const inputSeparator = fromFormat[fromFormat.length - 1]; and then use date.split(inputSeparator) instead. Closely related, your current logic ignores this separator part of fromFormat and only uses fromFormat to find positions of DD, MM, and YY/ YYYY, which means the function does not fully honor the requirement that fromFormat describes both order and separator of the input date. Once you fix the undefined variable and properly use the fromFormat separator when splitting, your reordering and year conversion logic (including the YYYYYY rules) should align well with the specification. As a minor improvement, consider clearer names like inputSeparator and outputSeparator to make the intent of each step easier to follow. Your core idea is solid, so with these changes you’ll have a robust solution that matches the task 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
function formatDate(date, fromFormat, toFormat) {
// write code here
const changeElement = toFormat[toFormat.length - 1];
const parts = date.split(baseElement);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

baseElement is used here but never defined anywhere in the function or file. You should derive the input separator from fromFormat (e.g., its last element) and use that variable in split to correctly parse the date and avoid a ReferenceError.

Comment thread src/formatDate.js Outdated
*/
function formatDate(date, fromFormat, toFormat) {
// write code here
const changeElement = toFormat[toFormat.length - 1];

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 correctly read the output separator from toFormat, but you never similarly read the input separator from fromFormat. According to the task, fromFormat also includes the separator, so you should use its last element to determine how to split the date string.

@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 is a critical bug in how you derive the input separator from fromFormat, and this prevents the function from correctly parsing the date string for all specified examples. Right now, baseElement is taken as fromFormat[toFormat.length - 1], which can be a format token like 'DD' instead of the actual separator, so date.split(baseElement) will not work as required for valid inputs such as ['YYYY','MM','DD','-']. You need to change the separator derivation so it always uses the last element of fromFormat, for example const baseElement = fromFormat[fromFormat.length - 1];, and then keep using that to split the incoming date string [CHECKLIST ITEM #4, CHECKLIST ITEM #5]. Once that is fixed, your logic for reordering parts and converting between YY and YYYY appears to meet the task requirements. As a minor improvement, consider renaming baseElement and changeElement to something like inputSeparator and outputSeparator to make the intent clearer, though this is not blocking. Overall, you are very close; fixing the separator handling so it matches the fromFormat specification should make your solution correct and complete.


✨ 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
*/
function formatDate(date, fromFormat, toFormat) {
// write code here
const baseElement = fromFormat[toFormat.length - 1];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

baseElement should be derived from the separator in fromFormat, which is always its last element. Using fromFormat[toFormat.length - 1] gives the wrong index whenever fromFormat and toFormat differ (e.g. ['YYYY','MM','DD','-'] and ['DD','MM','YYYY','.'] results in 'DD' instead of '-'). Change this to something like const baseElement = fromFormat[fromFormat.length - 1]; so the input date is split by the correct separator as required.

Comment thread src/formatDate.js Outdated
// write code here
const baseElement = fromFormat[toFormat.length - 1];
const changeElement = toFormat[toFormat.length - 1];
const parts = date.split(baseElement);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Once you fix the separator derivation above, this split will work correctly with the formats described in the task (the date string must be split using the separator defined in fromFormat). Right now, because baseElement can be a format token like 'DD' instead of the actual separator, parts will be incorrect and the function will fail to meet the specification.

@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 approving your solution because your formatDate implementation correctly handles all required behaviors: it derives the input separator from the last element of fromFormat, splits the input date with it, and uses the last element of toFormat to join the output string, matching the separator rules [CHECKLIST ITEM #4 and #5]. You accurately locate the day, month, and year positions based on fromFormat, reorder them for toFormat, and implement the YYYYYY conversions with the correct 20YY/19YY rule, which aligns with all specified examples. Your code focuses on the core logic without adding unnecessary validation, which is appropriate given the assumption that inputs are always well-formed.

One minor improvement opportunity is naming clarity—for example, a name like yearIndex instead of fromYearIndex could make the role of the variable more immediately obvious, but this is stylistic and your current naming is still understandable. Overall, this is solid, clean work that shows you carefully followed the requirements and previous feedback. Nice job implementing the rules precisely and keeping the solution straightforward and readable.


✨ 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