-
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 1 #1935
base: master
Are you sure you want to change the base?
solution 1 #1935
Conversation
src/formatDate.js
Outdated
const less = dateSplitted[i] < 30; | ||
const more = dateSplitted[i] >= 30; |
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 variables 'less' and 'more' are not descriptive of their purpose and can be confusing. Consider renaming them to reflect that they are used to determine the century when converting from 'YY' to 'YYYY'. For example, 'is20thCentury' and 'is21stCentury' would be more descriptive.
src/formatDate.js
Outdated
if (fromFormat[i] === 'YY' && toFormat.includes('YYYY') && less) { | ||
objectDateFormat.YYYY = '20' + dateSplitted[i]; | ||
} else if (fromFormat[i] === 'YY' && toFormat.includes('YYYY') && more) { | ||
objectDateFormat.YYYY = '19' + dateSplitted[i]; |
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 determining the century when converting from 'YY' to 'YYYY' is not entirely correct. The condition should check the actual year value, not the index of the date part. Also, the current logic does not handle the case where 'YY' is exactly '30'. You should compare the year directly with '30' to decide whether to prepend '19' or '20'.
src/formatDate.js
Outdated
if (fromFormat[i] === 'YYYY' && toFormat.includes('YY')) { | ||
objectDateFormat.YY = dateSplitted[i][2] + dateSplitted[i][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 conversion from 'YYYY' to 'YY' is correct, but it's better to use string slicing instead of indexing for readability and to avoid potential errors if the format changes. You can use 'dateSplitted[i].slice(-2)' to get the last two digits of the year.
src/formatDate.js
Outdated
} else { | ||
objectDateFormat[fromFormat[i]] = dateSplitted[i]; |
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 'else' block will incorrectly overwrite the 'YYYY' key in 'objectDateFormat' if both 'YYYY' and 'YY' are present in 'fromFormat'. This block should only execute if the current 'fromFormat[i]' is neither 'YY' nor 'YYYY'.
src/formatDate.js
Outdated
for (let i = 0; i < toFormat.length - 1; i++) { | ||
if (i > 0) { | ||
resultDate += toFormat[toFormat.length - 1]; | ||
} | ||
|
||
resultDate += objectDateFormat[toFormat[i]]; | ||
} |
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 loop for constructing the result date string is correct, but it's important to ensure that 'objectDateFormat' contains all the necessary keys before attempting to access them. Otherwise, you may encounter undefined values. Consider adding a check or default value for each key in 'objectDateFormat'.
src/formatDate.js
Outdated
const less = dateSplitted[i] < 30; | ||
const more = dateSplitted[i] >= 30; |
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 variables 'less' and 'more' are not necessary and can lead to confusion. The comparison should be done directly in the if conditions when checking the year format conversion. Additionally, the variable names 'less' and 'more' do not clearly indicate their purpose. Consider using more descriptive names or performing the checks directly in the if statements.
src/formatDate.js
Outdated
if (fromFormat[i] === 'YY' && toFormat.includes('YYYY') && less) { | ||
objectDateFormat.YYYY = '20' + dateSplitted[i]; | ||
} else if (fromFormat[i] === 'YY' && toFormat.includes('YYYY') && more) { | ||
objectDateFormat.YYYY = '19' + dateSplitted[i]; |
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 from 'YY' to 'YYYY' is not entirely correct. The condition should check the actual year value, not just whether it is less than 30. The current implementation will not work correctly for all cases, such as when the year part is '00'. You should parse the year part as an integer and then apply the logic to determine the correct century.
src/formatDate.js
Outdated
if (fromFormat[i] === 'YYYY' && toFormat.includes('YY')) { | ||
objectDateFormat.YY = dateSplitted[i][2] + dateSplitted[i][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 conversion from 'YYYY' to 'YY' is correct, but you should ensure that 'dateSplitted[i]' is a string and has a length of 4 before accessing its characters to avoid potential errors.
src/formatDate.js
Outdated
} else { | ||
objectDateFormat[fromFormat[i]] = dateSplitted[i]; |
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 'else' block will incorrectly overwrite the 'YYYY' or 'YY' key in 'objectDateFormat' if both 'fromFormat' and 'toFormat' contain 'YYYY' or 'YY', respectively. You should only assign to 'objectDateFormat[fromFormat[i]]' if the current format part is not involved in a year conversion.
src/formatDate.js
Outdated
for (let i = 0; i < toFormat.length - 1; i++) { | ||
if (i > 0) { | ||
resultDate += toFormat[toFormat.length - 1]; | ||
} | ||
|
||
resultDate += objectDateFormat[toFormat[i]]; | ||
} |
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 loop for constructing the result date string does not handle the case where 'toFormat' does not include one of the date parts present in 'fromFormat'. This can lead to missing date parts in the result. You should ensure that all parts present in 'toFormat' are accounted for in 'objectDateFormat' before constructing the result string.
src/formatDate.js
Outdated
function formatDate(date, fromFormat, toFormat) { | ||
// write code here | ||
const SEPARATOR = fromFormat[fromFormat.length - 1]; | ||
const JOINER = toFormat[toFormat.length - 1]; | ||
const DATEPARTS = date.split(SEPARATOR); | ||
|
||
while (DATEPARTS.length > 1 && DATEPARTS[DATEPARTS.length - 1] === '') { | ||
DATEPARTS.pop(); | ||
} | ||
|
||
const DATEMAP = {}; | ||
|
||
fromFormat.forEach((part, index) => { | ||
DATEMAP[part] = DATEPARTS[index]; | ||
}); | ||
|
||
if (fromFormat.includes('YYYY') && toFormat.includes('YY')) { | ||
DATEMAP['YY'] = DATEMAP['YYYY'].slice(-2); | ||
} | ||
|
||
if (fromFormat.includes('YY') && toFormat.includes('YYYY')) { | ||
const FULLYEAR | ||
= parseInt(DATEMAP['YY']) < 30 | ||
? '20' + DATEMAP['YY'] | ||
: '19' + DATEMAP['YY']; | ||
|
||
DATEMAP['YYYY'] = FULLYEAR; | ||
} | ||
|
||
const FORMATTEDDATE = toFormat.map((part) => DATEMAP[part]); | ||
|
||
return FORMATTEDDATE.slice(0, -1).join(JOINER); | ||
} |
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 function formatDate
is missing input validation. It's important to check if the provided date
, fromFormat
, and toFormat
arguments are valid and in the expected format. This includes checking if the date
string matches the fromFormat
pattern, and if the fromFormat
and toFormat
arrays have the correct length and contain valid date part identifiers. Without these checks, the function may produce incorrect results or throw errors when encountering unexpected input.
src/formatDate.js
Outdated
const JOINER = toFormat[toFormat.length - 1]; | ||
const DATEPARTS = date.split(SEPARATOR); | ||
|
||
while (DATEPARTS.length > 1 && DATEPARTS[DATEPARTS.length - 1] === '') { |
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 loop that removes empty strings from the end of DATEPARTS
is unnecessary if the input is validated correctly. If the date string is always in the correct format, there should not be any empty strings after splitting.
src/formatDate.js
Outdated
|
||
const DATEMAP = {}; | ||
|
||
fromFormat.forEach((part, index) => { |
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 forEach
loop is used to populate the DATEMAP
object with date parts. However, there is no check to ensure that DATEPARTS[index]
exists. This could lead to undefined
values being assigned to DATEMAP
properties if DATEPARTS
has fewer elements than expected, which again highlights the need for input validation.
src/formatDate.js
Outdated
|
||
const FORMATTEDDATE = toFormat.map((part) => DATEMAP[part]); | ||
|
||
return FORMATTEDDATE.slice(0, -1).join(JOINER); |
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 slice(0, -1)
at the end of the FORMATTEDDATE
array is incorrect. This will remove the last element of the array, which should be a date part, not the separator. You should return the entire FORMATTEDDATE
array joined by JOINER
without slicing it.
function formatDate(date, fromFormat, toFormat) { | ||
// write code here | ||
const dateParts = date.split(fromFormat[3]); | ||
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]}`; | ||
} | ||
|
||
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)); | ||
break; | ||
|
||
case 'MM': | ||
formattedDate.push(month); | ||
break; | ||
|
||
case 'DD': | ||
formattedDate.push(day); | ||
} | ||
} | ||
|
||
return formattedDate.join(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.
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.
@@ -8,7 +8,50 @@ | |||
* @returns {string} | |||
*/ | |||
function formatDate(date, fromFormat, toFormat) { | |||
// write code here | |||
const dateParts = date.split(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.
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.
} | ||
|
||
if (fromFormat[i] === 'YY') { | ||
year = dateParts[i] < 30 ? `20${dateParts[i]}` : `19${dateParts[i]}`; |
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 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.
break; | ||
|
||
case 'YY': | ||
formattedDate.push(year.slice(-2)); |
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.
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.
function formatDate(date, fromFormat, toFormat) { | ||
// write code here | ||
const dateParts = date.split(fromFormat[3]); | ||
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]}`; | ||
} | ||
|
||
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)); | ||
break; | ||
|
||
case 'MM': | ||
formattedDate.push(month); | ||
break; | ||
|
||
case 'DD': | ||
formattedDate.push(day); | ||
} | ||
} | ||
|
||
return formattedDate.join(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.
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 overfromFormat
, you could use object destructuring to assignyear
,month
, andday
directly if the order of the date parts is known to be consistent with thefromFormat
array. - The second loop that builds
formattedDate
can be simplified by mapping overtoFormat
and applying the transformation directly within themap
function, which would eliminate the need for theswitch
statement and make the code more concise.
No description provided.