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 1 #1935

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion src/formatDate.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,50 @@
* @returns {string}
*/
function formatDate(date, fromFormat, toFormat) {
// write code here
const dateParts = date.split(fromFormat[3]);

Choose a reason for hiding this comment

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

It's assumed that the separator is always the last element in the fromFormat array. While this works for the given examples, it might be safer to explicitly search for the separator in the fromFormat array to avoid potential issues if the array structure changes.

let year;
let month;
let day;
const formattedDate = [];

for (let i = 0; i < fromFormat.length; i++) {
if (fromFormat[i] === 'YYYY') {
year = dateParts[i];
}

if (fromFormat[i] === 'YY') {
year = dateParts[i] < 30 ? `20${dateParts[i]}` : `19${dateParts[i]}`;

Choose a reason for hiding this comment

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

The condition dateParts[i] < 30 works because JavaScript performs type coercion, but it's comparing a string to a number, which can be misleading. It's better to explicitly convert dateParts[i] to a number before the comparison for clarity and to avoid any potential type coercion issues.

}

if (fromFormat[i] === 'MM') {
month = dateParts[i];
}

if (fromFormat[i] === 'DD') {
day = dateParts[i];
}
}

for (const format of toFormat) {
switch (format) {
case 'YYYY':
formattedDate.push(year);
break;

case 'YY':
formattedDate.push(year.slice(-2));

Choose a reason for hiding this comment

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

There is an implicit assumption that year will always be a string of at least two characters. While this should be the case for valid dates, adding a check to ensure that year has the expected length before slicing could prevent potential errors.

break;

case 'MM':
formattedDate.push(month);
break;

case 'DD':
formattedDate.push(day);
}
}

return formattedDate.join(toFormat[3]);
Comment on lines 10 to +54

Choose a reason for hiding this comment

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

The implementation of the formatDate function seems correct, but there are a few edge cases that are not handled. For example, if the fromFormat or toFormat arrays have elements other than 'YYYY', 'YY', 'MM', 'DD', or the separator, the function will not behave correctly. It would be beneficial to add validation to ensure that the input formats only contain valid format specifiers. Additionally, consider edge cases where the input date string does not match the expected fromFormat structure, which could lead to incorrect parsing and formatting.

Comment on lines 10 to +54

Choose a reason for hiding this comment

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

The implementation of the formatDate function seems correct. However, there are a couple of improvements that could be made for better readability and performance:

  • Instead of using a for loop to iterate over fromFormat, you could use object destructuring to assign year, month, and day directly if the order of the date parts is known to be consistent with the fromFormat array.
  • The second loop that builds formattedDate can be simplified by mapping over toFormat and applying the transformation directly within the map function, which would eliminate the need for the switch statement and make the code more concise.

}

module.exports = formatDate;
Loading