-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 #2682
base: master
Are you sure you want to change the base?
solution #2682
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.
Almost done:smiley:
src/arrayMethodJoin.js
Outdated
// write code here | ||
[].__proto__.join2 = function(separator = ',') { | ||
const currentArrayLength = this.length; | ||
const joinSeparator = separator + ''; |
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.
I propose to use a more readable string transform for it.
const joinSeparator = separator + ''; | |
const joinSeparator = String(separator); |
src/arrayMethodJoin.js
Outdated
this.forEach((item, index) => { | ||
/* const stringCharacter = item ?? '' is the best choice, | ||
but I couldn`t disable eslint for the nullish operator */ | ||
const stringCharacter = item === null || item === undefined |
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.
You can make it shorter using ==
console.log(undefined == null) // true
console.log(null == null) // true
console.log(false == null) // false
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.
using == is very dangerous for our karma)
src/arrayMethodJoin.js
Outdated
currentArrayLength - 1 === index | ||
? resultingString += stringCharacter | ||
: resultingString += stringCharacter + joinSeparator; |
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.
I think better to use if statement here because it makes our code more readable.
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.
GJ 🚀
But let's make your code clearer
src/arrayMethodJoin.js
Outdated
// write code here | ||
[].__proto__.join2 = function(separator = ',') { | ||
const currentArrayLength = this.length; | ||
const joinSeparator = String(separator); |
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 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.
@AndrMar1939 reply to it
src/arrayMethodJoin.js
Outdated
if (!currentArrayLength) { | ||
return resultingString; | ||
} |
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.
Do we really need this?
If length === 0 we will skip forEach
and also return resultingString
src/arrayMethodJoin.js
Outdated
if (currentArrayLength - 1 === index) { | ||
resultingString += stringCharacter; | ||
} else { | ||
resultingString += stringCharacter + joinSeparator; | ||
} |
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.
Do we really need else
here? Maybe we can simplify this and use only if
?
P.S. We do the same part of logic in if
and in else
block
src/arrayMethodJoin.js
Outdated
const joinSeparator = String(separator); | ||
let resultingString = ''; | ||
|
||
this.forEach((item, 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.
Using Array methods is prohibited in this task. Use loops, index access, and array length if needed.
src/arrayMethodJoin.js
Outdated
// write code here | ||
[].__proto__.join2 = function(separator = ',') { | ||
const currentArrayLength = this.length; | ||
const joinSeparator = String(separator); |
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.
@AndrMar1939 reply to it
resultingString += stringCharacter; | ||
resultingString += separator; |
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.
let's simplify this
No description provided.