patternjavascriptModerate
Removing duplicates from an array quickly
Viewed 0 times
quicklyremovingarrayfromduplicates
Problem
Array.prototype.unique = function() {
var a = [], // uniques get placed into here
b = 0; // counter to test if value is already in array 'a'
for ( i = 0; i < this.length; i++ ) {
var current = this[i]; // get a value from the original array
for ( j = 0; j < a.length; j++ ) { // loop and check if value is in new array
if ( current != a[j] ) {
b++; // if its not in the new array increase counter
}
}
if ( b == a.length ) { // if the counter increased on all values
// then its not in the new array yet
a.push( current ); // put it in
}
b = 0; // reset counter
}
this.length = 0; // after new array is finished creating delete the original array
for ( i = 0; i < a.length; i++ ) {
this.push( a[i] ); // push all the new values into the original
}
return this; // return this to allow method chaining
}I'm expecting this to be a slow checker especially because there is no sorting first. I'm interested in improving my sorting abilities etc so I thought I would get an early review.
Solution
You can use the
At the end you are replacing the array's content. It would be better to not mutate the original array, but to return the new array instead with unique elements:
The name
You should declare the loop variable
Finally, there is a much more elegant solution to this problem, using the reduce function:
UPDATE (to answer your follow-up question)
If you want the function to take a parameter to decide whether it should modify the original array, you could try something like this:
indexOf function to check if a value exists in an array. This would simplify your code greatly:Array.prototype.unique = function() {
var a = [];
for ( i = 0; i < this.length; i++ ) {
var current = this[i];
if (a.indexOf(current) < 0) a.push(current);
}
this.length = 0;
for ( i = 0; i < a.length; i++ ) {
this.push( a[i] );
}
return this;
}At the end you are replacing the array's content. It would be better to not mutate the original array, but to return the new array instead with unique elements:
Array.prototype.unique = function() {
var a = [];
for ( i = 0; i < this.length; i++ ) {
var current = this[i];
if (a.indexOf(current) < 0) a.push(current);
}
return a;
}The name
a is a very poor choice. unique would have been better.You should declare the loop variable
i using let, to limit its scope to the current block, so the code becomes:Array.prototype.unique = function() {
var unique = [];
for (let i = 0; i < this.length; i++) {
let current = this[i];
if (unique.indexOf(current) < 0) unique.push(current);
}
return unique;
}Finally, there is a much more elegant solution to this problem, using the reduce function:
Array.prototype.unique = function() {
return this.reduce(function(accum, current) {
if (accum.indexOf(current) < 0) {
accum.push(current);
}
return accum;
}, []);
}UPDATE (to answer your follow-up question)
If you want the function to take a parameter to decide whether it should modify the original array, you could try something like this:
Array.prototype.unique = function(mutate) {
var unique = this.reduce(function(accum, current) {
if (accum.indexOf(current) < 0) {
accum.push(current);
}
return accum;
}, []);
if (mutate) {
this.length = 0;
for (let i = 0; i < unique.length; ++i) {
this.push(unique[i]);
}
return this;
}
return unique;
}Code Snippets
Array.prototype.unique = function() {
var a = [];
for ( i = 0; i < this.length; i++ ) {
var current = this[i];
if (a.indexOf(current) < 0) a.push(current);
}
this.length = 0;
for ( i = 0; i < a.length; i++ ) {
this.push( a[i] );
}
return this;
}Array.prototype.unique = function() {
var a = [];
for ( i = 0; i < this.length; i++ ) {
var current = this[i];
if (a.indexOf(current) < 0) a.push(current);
}
return a;
}Array.prototype.unique = function() {
var unique = [];
for (let i = 0; i < this.length; i++) {
let current = this[i];
if (unique.indexOf(current) < 0) unique.push(current);
}
return unique;
}Array.prototype.unique = function() {
return this.reduce(function(accum, current) {
if (accum.indexOf(current) < 0) {
accum.push(current);
}
return accum;
}, []);
}Array.prototype.unique = function(mutate) {
var unique = this.reduce(function(accum, current) {
if (accum.indexOf(current) < 0) {
accum.push(current);
}
return accum;
}, []);
if (mutate) {
this.length = 0;
for (let i = 0; i < unique.length; ++i) {
this.push(unique[i]);
}
return this;
}
return unique;
}Context
StackExchange Code Review Q#60128, answer score: 19
Revisions (0)
No revisions yet.