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 #2162

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

solution #2162

wants to merge 1 commit into from

Conversation

Ilya-Ilchuk
Copy link

No description provided.

Copy link

@maxsabo maxsabo left a comment

Choose a reason for hiding this comment

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

Good job!

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.

Great job on implementing the formatDate function! 🎉 Your solution meets the task requirements and handles the conversion of date formats effectively. While there are some assumptions about the position of separators and century logic for years, these do not critically impact the functionality for the current requirements. Keep up the good work, and consider making these assumptions more flexible in future iterations to enhance the robustness of your code. 😊

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

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

Choose a reason for hiding this comment

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

The separator is assumed to be at index 3 of the fromFormat array. Ensure that this assumption is valid for all expected input formats. If the separator is not always at this position, the logic may need to be adjusted.

Comment on lines +19 to +23
if (fromFormat.includes('YY')) {
oldDateFormat['YYYY'] =
parseInt(oldDateFormat['YY'], 10) < 30
? '20' + oldDateFormat['YY']
: '19' + oldDateFormat['YY'];

Choose a reason for hiding this comment

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

The logic for converting 'YY' to 'YYYY' assumes that any year less than 30 is in the 2000s, and any year 30 or greater is in the 1900s. This assumption may not hold for all use cases, especially if the input date could be from a different century. Consider making this logic more flexible or configurable.

oldDateFormat['YY'] = oldDateFormat['YYYY'].slice(-2);
}

const newSeparator = toFormat[3];

Choose a reason for hiding this comment

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

Similar to the fromFormat, the separator is assumed to be at index 3 of the toFormat array. Ensure this assumption is valid for all expected output formats.

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