-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 #2163
base: master
Are you sure you want to change the base?
Solution #2163
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job👍
There was a problem hiding this 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 constraints effectively. While there are a few areas for improvement, such as handling unexpected year values and making the separator identification more robust, these do not critically impact the functionality of your code. Keep up the good work, and consider these suggestions for future enhancements! 😊
✨ 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]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code assumes that the separator is always at the fourth position in the fromFormat
array. This assumption might not hold true for all date formats. Consider a more flexible approach to determine the separator.
dateObj['YYYY'] = year < 30 ? `20${dateObj['YY']}` : `19${dateObj['YY']}`; | ||
} | ||
|
||
const newSeparator = toFormat[3]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the issue with fromFormat
, the code assumes the separator is at the fourth position in the toFormat
array. This might not be accurate for all formats. Consider a more robust method to identify the separator.
if (fromFormat.includes('YYYY') && toFormat.includes('YY')) { | ||
dateObj['YY'] = dateObj['YYYY'].slice(-2); | ||
} else if (fromFormat.includes('YY') && toFormat.includes('YYYY')) { | ||
const year = parseInt(dateObj['YY'], 10); | ||
|
||
dateObj['YYYY'] = year < 30 ? `20${dateObj['YY']}` : `19${dateObj['YY']}`; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for converting between 'YYYY' and 'YY' formats is correct, but it doesn't account for invalid or unexpected year values. Consider adding validation to ensure the year values are within a reasonable range.
No description provided.