Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solution #2171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Solution #2171

wants to merge 1 commit into from

Conversation

RomanHasiuk
Copy link

No description provided.

Copy link

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

Choose a reason for hiding this comment

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

Your implementation of the formatDate function is well done and meets the task requirements effectively! 🎉 While there are a few areas for potential improvement, such as ensuring that all format parts are correctly mapped and handling separators more dynamically, these do not detract from the core functionality of your solution. Keep up the great work, and consider these suggestions for future enhancements to make your code even more robust! 😊

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

@@ -8,7 +8,32 @@
* @returns {string}
*/
function formatDate(date, fromFormat, toFormat) {
// write code here
const arrDate = date.split(fromFormat[3]);

Choose a reason for hiding this comment

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

The use of fromFormat[3] to split the date string assumes that the separator is always at index 3. This might not be the case if the format changes. Consider dynamically determining the separator or passing it as a separate parameter.

@@ -8,7 +8,32 @@
* @returns {string}
*/
function formatDate(date, fromFormat, toFormat) {
// write code here
const arrDate = date.split(fromFormat[3]);
const slash = toFormat[3];

Choose a reason for hiding this comment

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

Similarly, toFormat[3] is used as the separator for joining the new date format. Ensure that this index is correct for all possible formats or handle separators separately.

Comment on lines +16 to +18
for (let i = 0; i < fromFormat.length - 1; i++) {
dateMap[fromFormat[i]] = arrDate[i];
}

Choose a reason for hiding this comment

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

The loop iterates over fromFormat.length - 1, which might miss the last element if the format array is longer than expected. Consider iterating over the entire array to ensure all parts are mapped.

Comment on lines +20 to +34
for (let i = 0; i < toFormat.length - 1; i++) {
const part = toFormat[i];

if (part === 'YY' && dateMap['YYYY']) {
newFormat.push(dateMap['YYYY'].slice(-2));
} else if (part === 'YYYY' && dateMap['YY']) {
const year = parseInt(dateMap['YY'], 10);

newFormat.push(
year < 30 ? `20${year.toString().padStart(2, '0')}` : `19${year}`,
);
} else {
newFormat.push(dateMap[part]);
}
}

Choose a reason for hiding this comment

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

The logic for handling 'YY' and 'YYYY' conversions is correct, but ensure that dateMap contains the necessary keys before accessing them to avoid potential undefined errors.

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