Mircode
Mircode

Reputation: 736

Leaky abstraction with setters and getters in js/ts

I am working on a game. Because I don't want to marry frameworks, I wrote an abstraction for the graphics engine and am now trying to shoehorn three.js into it.

I am working with gl-matrix for vector math (using vec3, typed arrays), three.js has its own vector classes (using {x:number, y:number, z:number}). My interface for a 3d object looks as follows:

interface Thing{
    ...
    position:vec3
}

and my implementation looks like this:

class ThreeJsThing implements Thing{
    ...
    set position(p:vec3){
        this.mesh.position.set(p[0], p[1], p[2]) // [,,] -> {xyz}
    }
    get position(){
        let p = this.mesh.position
        return vec3.fromValues(p.x, p.y, p.z) // {xyz} -> [,,]
    }
}

Now it's all fun and games until someone tries to do:

let myThing = new ThreeJsThing()
myThing.position[1] = 5 // only changes a temporary copy that is returned by the getter

And with gl-matrix this is especially a problem because it uses output parameters for performance reasons, internally doing that array assignment above:

vec3.add(myThing.position, a, b)

I know how to work around this problem because I am (now) aware of it, but other contributors are probably also going to choke on this, especially since it fails silently. Just from a clean code point of view, I can't really pinpoint where I used anti-patterns or bad design or something. I find ThreeJsThing to be as faithful to the interface as possible. I have some suspicions:

Any advice appreciated.

EDIT:

While I do appreciate remarks on the context, I provided it to show what lead me to this problem. I consider this to be a broader problem that I would like to pinpoint so I know how and when to avoid it in other contexts.

Judging from the comments, the crux seems to be that I should not use get when it returns a mutable copy, since it looks like a regular writable member from the outside. Are there more gotchas to be aware of with getters and setters?

I would consider this on a level with Python's mutable defaults, I find it easy to run into and not very obvious... I wonder why this isn't mentioned anywhere...

Upvotes: 0

Views: 112

Answers (1)

Mircode
Mircode

Reputation: 736

Solution suggested in the comments:

// gl-matrix
type vec3 = [number, number, number] | Float32Array;
type ReadonlyVec3 = readonly [number, number, number] | Float32Array;
// three.js
type Vector3 = {x:number, y:number, z:number}

interface Thing{
    setPosition(p:ReadonlyVec3): void
    getPosition():ReadonlyVec3
}

class ThreeJsThing implements Thing{
    private position:Vector3 = {x:0, y:0, z:0}
    setPosition(p:ReadonlyVec3){
        // actually this.position.set(p[0], p[1], p[2])
        this.position.x = p[0]
        this.position.y = p[1]
        this.position.z = p[2]
    }
    getPosition(){
        // actually return vec3.fromValues(...) as const
        return [
            this.position.x,
            this.position.y,
            this.position.z
        ] as const
    }
}

let thing = new ThreeJsThing()
let position = [1, 2, 3] as vec3
thing.setPosition(position)
console.log(thing.getPosition())
console.log(thing.getPosition()[1])
// position = thing.getPosition() // Type 'readonly [...]' is not assignable to type 'vec3'.
// thing.getPosition()[1] = 5 // Cannot assign to '1' because it is a read-only property.

Here is another solution that was posted by KLASANGUI, got downvoted into oblivion and then deleted. Use with caution, it probably opens a whole nother can of worms and blows up at three more places but I consider it absolutely valid and a beautiful JavaScript flex. It completely solves every problem I stated without syntactic sugar compromises.

// gl-matrix
type vec3 = [number, number, number] | Float32Array;
type ReadonlyVec3 = readonly [number, number, number] | Float32Array;
// three.js
type Vector3 = { x: number, y: number, z: number }

interface Thing {
    position: vec3
}

class ThreeJsThing implements Thing {
    #position: Vector3 = { x: 0, y: 0, z: 0 }

    set position(p: vec3) {
        this.#position.x = p[0]
        this.#position.y = p[1]
        this.#position.z = p[2]
    }

    get position() {
        let p = this.#position
        const accessor = {
            length: 3,
            set 0(x:number) { p.x = x },
            get 0() { return p.x },
            set 1(y:number) { p.y = y },
            get 1() { return p.y },
            set 2(z:number) { p.z = z },
            get 2() { return p.z }
        };
        Object.setPrototypeOf(accessor, Array.prototype);
        return accessor as vec3
    }
}

let thing = new ThreeJsThing()
let position = [1, 2, 3] as vec3
thing.position = position
thing.position[1] = 5
position = thing.position
console.log(position)

Upvotes: -1

Related Questions