Extend native `Array` instances

Question

I need/want to extend created array instance, to add extra methods that i see as useful.

class.labels.js

const {
    find,
    filter,
    value,
    key
} = require("./labels.js");

module.exports = class Labels extends Array {

    constructor(...args) {
        super(...args);
    }

    find(...args) {
        return find(this, ...args);
    }

    filter(...args) {
        return filter(this, ...args);
    }

    value(...args) {
        return value(this, ...args);
    }

    key(...args) {
        return key(this, ...args);
    }

};

labels.js

function match(label, filter) {

    let [fk, fv] = filter.split("=");
    let [lk, lv] = label.split("=");

    if (fv === "*") {
        return fk === lk;
    }

    if (fk === "*") {
        return fv === lv;
    }

    return label === filter;

}


function find(arr, filter) {
    return Array.prototype.find.call(arr, (label) => {
        return match(label, filter);
    });
}


function filter(arr, filter) {
    return Array.prototype.find.call(arr, (label) => {
        return match(label, filter);
    });
}


function value(arr, key) {
    return find(arr, `${key}=*`)?.split("=")[1];
}


function key(arr, val) {
    return find(arr, `*=${val}`)?.split("=")[0];
}

module.exports = {
    match,
    find,
    filter,
    value,
    key
};

example.js

const Labels = require("./class.labels.js");
const labels = new Labels();


labels.push("key=value");
labels.push("foo=bar");
labels.push("bar=baz");
labels.push("key1=value")

console.log(labels);


console.log(labels.find("foo=*"));


console.log(labels.map((s) => {
    return String.prototype.toUpperCase.call(s);
}));


console.log(labels.key("value"));
console.log(labels.value("key1"));

Is this praxis considered ok?

Answer 1

function find(arr, filter) {
...
function filter(arr, filter) {

DRY. I can't say I'm pleased with this pair of functions. They are literally the same thing, byte for byte. Pick one and go with it. Don't pollute the IDE autocomplete menu with multiple spellings for same concept. Don't clutter the documentation of your public API.

Maybe find is culturally appropriate, since we're calling into Array's find which callers are already familiar with. Maybe filter is better, as it suggests zero or more results. You are the API designer. Choose a winner and run with it.

(Imagine there was some backward compatibility reason why the V2 filter needs to be just like the V1 find. That's unfortunate, but still no reason for copy-n-paste code. Choose one to be canonical, and have the other call it.)

---

Apparently it would be Bad to have a data value that includes the = equal sign. (Similarly for keys, but I'm willing to believe that comes up less often.) We really need to have a comment or other documentation which explains that.

More generally, there seems to be some business concept of a Label which would benefit from documentation that introduces the notion. Maybe there's a specification for what valid labels look like.

---

In example.js we find some demo calls, which is helpful documentation, it shows what's expected.

But it's a far cry from using Jest or similar unit test framework. Automated tests should be self-evaluating, they should "know" the right answer, so they can report Green / Red bar results. The console logging we see here assumes that the human eyeballing the results will know the right answer, and will usefully respond when the wrong answer is displayed.

---

Overall?

This module seems to have a "find vs filter?" issue that still needs to be resolved. With one of them deleted (or at least deprecated), it should be ready to ship.

We still need to decide whether we're commited to automated testing of this codebase. As it stands, I would be reluctant to delegate or accept maintenance tasks for it.

Answer 2

Do not extend js Array. Like never. Array is type that is a collection capable of containg inhomogeneous elements. Your Labels class, on other hand, expects elements of type string only. Inheriting the interface of Array for this purpose breaks Liskov substitution principle, to say at least. You want those 4 methods on your class and that's ok, but it's not OK to expose all the other methods of Array in your class as well. Prefer composition over inheritance. Just give your class a property of type Array and make your methods work over that Array instance.