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

Extending Object.prototype in JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptobjectprototypeextending

Problem

I find the default Javascript extremely poor on useful functions. There are some nice libraries, but somehow I always need something I can't find. Currently, I'd need a method removing all matching properties from an object. And I really mean removing, not creating a filtered copy. I've found similar methods in both jquery and underscore, but no exact match.

That's a long introduction, why I want to give it a try. As both $ and _ are taken, I'm extending the Object.prototype which should be fine. So this is the code:

function removeMatching(predicate) {
    for (var key in this) {
        if (!Object.prototype.hasOwnProperty.call(this, key)) continue;
        if (predicate(key, this[key])) delete this[key];
    };
};

Object.defineProperty(Object.prototype, 'removeMatching', {
    value: removeMatching,
    writable: false,
    configurable: false,
    enumerable: false
});


It's all gets enclosed in a closure to prevent namespace pollution. I don't care much about speed as this can be improved later (but tell me). I don't care about style (it's fixed) not about compatibility with old crap. I do care about all problems possible in modern browsers.

It's to be used like

var o = {a: 1, b: 2, c: 3};
o.removeMatching(function(k, v) {return k === "a" || v === 2});
// now o is {c: 3}

Solution

There are extraneous semicolons (null statements) in your code. Semicolons do not belong after function declarations and for loops.


I'm extending the Object.prototype which should be fine.

I would argue that it's not fine. While it's unlikely to cause undesired side effects, breaking encapsulation and polluting all objects just for the sake of being able to write foo.removeMatching(bar) instead of removeMatching(foo, bar) seems like bad form.

if (!Object.prototype.hasOwnProperty.call(this, key)) continue;


Since this is an Object by definition, because you are attaching this to Object.prototype, there is no need to write it like that. This would achieve the same thing (unless you are shadowing hasOwnProperty in which case you have bigger problems):

if (!this.hasOwnProperty(key)) continue;


However there's no need to write this line in the first place. If the object doesn't have a property as its own property, attempting to delete it will do nothing.


It's all gets enclosed in a closure to prevent namespace pollution.

This also seems unnecessary, why not just write it like this:

Object.defineProperty(Object.prototype, 'removeMatching', {
    value: function (predicate) {
        for (var key in this) {
            if (predicate(key, this[key])) {
                delete this[key];
            }
        }
    },
    writable: false,
    configurable: false,
    enumerable: false
});

Code Snippets

if (!Object.prototype.hasOwnProperty.call(this, key)) continue;
if (!this.hasOwnProperty(key)) continue;
Object.defineProperty(Object.prototype, 'removeMatching', {
    value: function (predicate) {
        for (var key in this) {
            if (predicate(key, this[key])) {
                delete this[key];
            }
        }
    },
    writable: false,
    configurable: false,
    enumerable: false
});

Context

StackExchange Code Review Q#55166, answer score: 6

Revisions (0)

No revisions yet.