HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Removing duplicates from an array quickly

Submitted by: @import:stackexchange-codereview··
0
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 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.