Pr 5 #5
Pr 5 #5
Conversation
| array.forEach((currentElement, currentIndex) => { | ||
| array.slice(currentIndex + 1).forEach((elementToCompare) => { | ||
| if (elementToCompare === currentElement) { | ||
| isAllUnique = false; |
There was a problem hiding this comment.
We can add a return if isAllUnique = false instead to make the program run more efficiently
| array.forEach((currentElement, currentIndex) => { | ||
| array.slice(currentIndex + 1).forEach((elementToCompare) => { | ||
| if (elementToCompare === currentElement) { | ||
| isAllUnique = false; |
There was a problem hiding this comment.
You may want to consider exiting loop if isAllUnique==false. Otherwise, it will loop through the whole array.
| array.forEach((currentElement, currentIndex) => { | ||
| array.slice(currentIndex + 1).forEach((elementToCompare) => { | ||
| if (elementToCompare === currentElement) { | ||
| isAllUnique = false; |
There was a problem hiding this comment.
If you add a return as soon as you find a duplicate, it will be more efficient because it won't have to keep searching through the rest of the array
| let isAllUnique = true; | ||
|
|
||
| array.forEach((currentElement, currentIndex) => { | ||
| array.slice(currentIndex + 1).forEach((elementToCompare) => { |
There was a problem hiding this comment.
Consider limiting the number of loops within loops to improve time efficiency
| @@ -0,0 +1,16 @@ | |||
|
|
|||
There was a problem hiding this comment.
Add a comment to describe what type of uniqueness you are looking for (unique characters? unique array items?)
| array.forEach((currentElement, currentIndex) => { | ||
| array.slice(currentIndex + 1).forEach((elementToCompare) => { | ||
| if (elementToCompare === currentElement) { | ||
| isAllUnique = false; |
There was a problem hiding this comment.
I feel like we should be returning isAllUnique right away if it is false so that the code doesn't have to continue through the entire array before returning.
| @@ -0,0 +1,14 @@ | |||
|
|
|||
| const isAllUniqueElements = (array) => { | |||
There was a problem hiding this comment.
Could we rename to areAllElementsUnique? Might read a little more smoothly ...
| }); | ||
|
|
||
| return isAllUnique; | ||
| }; |
| array.forEach((currentElement, currentIndex) => { | ||
| array.slice(currentIndex + 1).forEach((elementToCompare) => { | ||
| if (elementToCompare === currentElement) { | ||
| isAllUnique = false; |
There was a problem hiding this comment.
Returns true for empty array -- should it return null instead?
| array.forEach((currentElement, currentIndex) => { | ||
| array.slice(currentIndex + 1).forEach((elementToCompare) => { | ||
| if (elementToCompare === currentElement) { | ||
| isAllUnique = false; |
There was a problem hiding this comment.
Can we refactor to exit the loop as soon as it is false?
| const isAllUniqueElements = (array) => { | ||
| let isAllUnique = true; | ||
|
|
||
| array.forEach((currentElement, currentIndex) => { |
There was a problem hiding this comment.
good function to check for unique elements
This method determines if each element is unique.