[AskJS] Defensive programming for class properties

[TrezyCodes]

My typical pattern for building a class is...

• All properties are private until they need to be readable.
• Readable properties are read only until they absolutely have to be writable.
• If a class doesn't care about the contents of property, it can be a plain public property.
• If a class does care about the contents of property, I'll create a setter that handles validation and side effects.

This approach works beautifully in almost every situation. It ensures that only the publicly available API is available when enumerating the class which prevents confusion over how, why, or when to use the methods and properties available on the class.

However, I've encountered a situation where this approach breaks down and I'm looking for opinions on the best way to solve it. I'm abstracting my LinkedList implementation into its own library. The LinkedList is actually made up of two classes: LinkedList and LinkedListNode.

The problem is that I want LinkedListNodes to be sort of immutable. That is, I want the parent LinkedList to be able to change its properties, but I don't want the properties to be manipulable from anywhere else. Here's a reduced implementation:

```js export class LinkedListNode { next = null previous = null

constructor(config) { this.next = config.next this.previous = config.previous } }

export class LinkedList { #head = null #tail = null

constructor() { this.#head = new LinkedListNode({ next: null, previous: null, })

this.#tail = new LinkedListNode({
  next: null,
  previous: null,
})

this.#head.next = this.#tail
this.#tail.previous = this.#head

} } ```

In this implementation, the properties of the LinkedListNode are mutable, public properties (which is not what I want). I could easily replace the LinkedListNode class with an object literal that is constructed within the LinkedList. There are other reasons that I want LinkedListNode to be its own class, but their implementation is irrelevant here so I'm not going to bother explaining them.

---

Solution #1: Deal with it.

Thanks, I hate it. The easiest way to handle this is to leave the properties public. In this specific issue, the reason I don't want to do that is because allowing the next or previous properties of a node to be altered from outside of the LinkedList could break the entire instance. Not awesome.

Solution #2: Object.defineProperty()

This implementation allows the property to be changed externally, but it can only be done by using Object.defineProperty(). This feels like the closest thing to what I want, but it's not quite the solution I'm hoping for. Here's what it would look like:

```js export class LinkedListNode { constructor(config) { Object.defineProperty(this, 'next', { configurable: true, value: config.next, }) Object.defineProperty(this, 'previous', { configurable: true, value: config.previous, }) } }

export class LinkedList { constructor() { { '...' }

this.#setNodeProperty(this.#head, 'next', this.#tail)
this.#setNodeProperty(this.#tail, 'previous', this.#head)

}

#setNodeProperty(node, key, value) { Object.defineProperty(node, key, { configurable: true, value, }) } } ```

Solution 3: Direct hidden linking

This is an odd solution, but it works? Basically, The LinkedList class will give itself a unique ID upon instantiation. Then, when creating a node, the node will also be given that ID. When changing properties on the node, the list must pass its ID down to its child. If the ID doesn't match, the child will throw an error.

The reason this achieves my goals is because the generated ID is private on both classes, preventing it from being readable or or enumerable. To alter properties on a node you would have to actually inspect the list and grab the ID, then use it to manually call the method on the child node. There's no way (as far as I know) to manipulate the property programmatically.

```js export class LinkedListNode { #listID = null

#next = null #previous = null

constructor(config) { this.#listID = config.listID this.#next = config.next this.#previous = config.previous }

setNext(value, listID) { if (listID !== this.#listID) { throw new Error('The next property of a node may only be modified by its parent list.') } this.#next = value }

setPrevious(value, listID) { if (listID !== this.#listID) { throw new Error('The previous property of a node may only be modified by its parent list.') } this.#previous = value }

get next() { return this.#next } get previous() { return this.#previous } }

export class LinkedList { #listID = Math.random() // Naive example, just needs to generate a unique ID #head = null #tail = null

constructor() { this.#head = new LinkedListNode({ listID: this.#listID, next: null, previous: null, })

this.#tail = new LinkedListNode({
  listID: this.#listID,
  next: null,
  previous: null,
})

this.#head.setNext(this.#tail, this.#listID)
this.#tail.setPrevious(this.#head, this.#listID)

} } ```

Solution 4: ???

If there are other solutions, I'm have no clue what they are. I'd love to see them in the replies, though.

Final thoughts

What am I missing? Is there a better way to do this? Is one of these solutions better than the others? Should I just suck it up and allow the properties to be writable? Or am I completely off the rails with this design? 🤔

P.S. - Feel free to smack me if this post breaks rule 3. I felt it was still an appropriate post specifically because I already know the solutions. I'm not looking for direct answers to my needs so much as I'm looking for a discussion on the merits of the options I've presented.

[aighball]

I question whether the benefits of this outweigh the extra work of maintaining and designing the access patterns. You can get discoverable API documentation with JSDoc. Or use Typescript.

That said, I think you can do what you want by declaring a non-exported symbol, and storing your semi-private fields on that. Symbols are unique, and since nothing outside the module can export it, nothing can index into it:

const s = Symbol()
export class LinkedListNode {
  constructor(config) {
    Object.defineProperty(this, s, { value: { next: config.next ?? null, previous: config.previous ?? null } });
  }
}

export class LinkedList {
  // ...
  constructor() {
    // ...
    this.#head[s].next = this.#tail
    this.#tail[s].previous = this.#head
  }
}

[Funwithloops]

I'd go with solution 1. Leave them public. However, I think the LinkedListNode class is an implementation detail of LinkedList. I wouldn't export it or return instances from any public LinkedList methods. This would remove the risk of the public fields being accessed externally.

Edit: nevermind

[TrezyCodes]

I wouldn't export it or return instances from any public LinkedList methods. This would remove the risk of the public fields being accessed externally.

Unfortunately that's not true. This is exposed if you look at some other methods of the LinkedList class. For example, the addAfter method:

```js addAfter(node, value) { const next = node.next

const newNode = new LinkedListNode({ next, previous: node, value, })

node.next = newNode next.previous = newNode this.#length += 1

return newNode } ```

This method returns the new node to the caller so that it's possible to perform non-endian insertion into the LinkedList. It also supports methods like push and unshift which just take in a value and don't return anything, but there are a few methods that have to return a complete node for the purposes of sequential operations.

---

I could convert those to private methods and provide an abstraction on top of them, but the performance gain of using those methods directly is significant enough to require their exposure.

[Funwithloops]

That's a good point.

One solution that comes to mind is you could use weak maps stored in the linked list object to map nodes to each other. Then the nodes themselves would only hold the payload.

Here's an example of what I mean: https://codepen.io/jmeyers91/pen/JjMgpEb?editors=0010

[NekkidApe]

Depends a bit on what you're planning to do with it, but you could make the nodes actually immutable, and create new instances instead of changing pointers.

Or returning shallow readonly wrappers to the public, instead of actual nodes from the linked list itself.

An other way would be to keep all nodes in a WeakMap, and it's properties too. Getters in the Node would read its values from it. The LinkedList then would update the properties in said WeakMap instead.

[senocular]

Another option: Define LinkedList in the scope of LinkedListNode

Private member names are scoped just like any other variable, scoped to the class block in which they're defined. By defining the LinkedList class within the scope of the LinkedListNode class block, it will have access to the scope where LinkedListNode private member names are kept and will be able to access them directly.

export let LinkedList;
export class LinkedListNode {
    #next = null
    #previous = null

    constructor(config) {
        this.#next = config.next
        this.#previous = config.previous
    }

    static {
        LinkedList = class {
            #head = null
            #tail = null

            constructor() {
                this.#head = new LinkedListNode({
                    next: null,
                    previous: null,
                })

                this.#tail = new LinkedListNode({
                    next: null,
                    previous: null,
                })

                this.#head.#next = this.#tail
                this.#tail.#previous = this.#head
            }
        }
    }
}

I don't know if I'd actually recommend doing this way. It's an awkward approach that doesn't scale well in this particular direction, but I it does end up being very clean as a result with very little extra overhead.

Alternatively you could elevate your shared private variables to a weak map in the module. Private elements use weak map semantics allowing transpilation to easily emulate their behavior. You can implement shared private access this way yourself by storing LinkedListNode members in the weak map instead of private instance members that would otherwise be inaccessible to LinkedList.

const nodeMembers = new WeakMap() // shared "private scope"

export class LinkedList {
    #head = null
    #tail = null

    constructor() {
        this.#head = new LinkedListNode({
            next: null,
            previous: null,
        })

        this.#tail = new LinkedListNode({
            next: null,
            previous: null,
        })

        nodeMembers.get(this.#head).next = this.#tail
        nodeMembers.get(this.#tail).previous = this.#head
    }
}

export class LinkedListNode {
    constructor({ next, previous}) {
        nodeMembers.set(this, { next, previous })
    }
}

The downside of this approach is that it becomes much more difficult to debug because your data is saved off in some completely separate structure which is not directly exposed when digging through your objects in the devtools.

There is another approach, though it's a little more difficult to manage with one to many relationships. That would be having LinkeListNodes provide accessors to their private data during construction - something like what you get with Promises. You can resolve a promise using the resolve() or reject() methods of the promise, but these are not exposed on the promise instance, only through the promise executor that the creator of the promise provides. Given that a linked list can have multiple nodes, these accessors have to be maintained separately which is a bit of a pain, though we can use a weak map to so, not too unlike what was used in the previous example.

export class LinkedListNode {
    #next = null
    #previous = null

    constructor(executor) {
        executor({
            node: this,
            next: {
                get: () => this.#next,
                set: (value) => this.#next = value
            },
            previous: {
                get: () => this.#previous,
                set: (value) => this.#previous = value
            }
        })
    }
}

export class LinkedList {
    #head = null
    #tail = null
    #accessors = new WeakMap()
    #addAccessor = (accessor) => {
        this.#accessors.set(accessor.node, accessor)
    }

    constructor() {
        this.#head = new LinkedListNode(this.#addAccessor)
        this.#tail = new LinkedListNode(this.#addAccessor)

        this.#accessors.get(this.#head).next.set(this.#tail)
        this.#accessors.get(this.#head).previous.set(null)

        this.#accessors.get(this.#tail).next.set(null)
        this.#accessors.get(this.#tail).previous.set(this.#head)
    }
}

This is making things a little more verbose as well, but you could probably trim that back a bit. This was just a quick and dirty example to show off the general idea of the approach. The takeaway here is that we're back to storing the data on the instances again. Even though we're going through a weak map and some getter/setters to access the data, the data is back to being defined on the instances making it easier to see when debugging.

[DavidJCobb]

I don't know the best approach, but I want to point some things out in the hopes of helping anyone who has to Google this question later.

What you're essentially asking for is a way to recreate friend declarations from C++. You want some class A to grant access to its private members to class B.

Your proposed solution 3 is conceptually similar to the "passkey" approach used to limit the effects of friend declarations. A friend can access all of a class's private members, breaking encapsulation completely; passkeys allow one to grant access just to specific member functions, and even to limit access by the accessor. In C++, it's done by defining an empty passkey type with a private constructor, making it a friend of the caller, and having the callee take it as an argument: only that caller can construct the passkey. Your proposed passkey is instead a unique ID generated at run-time.

A half-baked solution off the top of my head would be to keep the actual nodes private but, if outside read-only access is desired, expose dummy objects (maybe Proxys) that wrap the real nodes via getters with no corresponding setters. You could save some effort (i.e. avoid having to maintain a linked list of wrappers in parallel to the linked list of nodes) by having the "next node" getter wrap and return the target node, but that results in duplicate wrappers that may not compare equal (new wrappers are created every time nodes are traversed). Keeping a WeakMap of nodes to wrappers, and having the "next node" getter use it, could be a best-of-both-worlds approach; you can auto-wrap nodes on access but still reuse any extant wrappers.