Is it memory inefficient to instantiate classes in a getter method? What OOP design prevents this?

I have a getter method that instantiates classes from deserialized data. I suspect this is a terribly inefficient way of doing things

class Container {
  get content() {
    let contentList = []
    for (let c of this._content) { // this._content is an array of objects
      contentList.push(new ContentTypes[c.type](c)) //ContentTypes is a mapping between type names and classes
    }
    return contentList
  }
}

What's the appropriate design pattern in OOP land?

Whether the code is in a getter or a method, the efficiency doesn't change. But what changes is the expectation of the person writing code using your class.

I don't expect accessing a property on an object to incur much overhead. So when using an instance of Container, if I see a contents property, I'm not going to expect it to do a lot of work. Yours does (a bit).

I would either:

1. Keep a copy of what you return (making what you return immutable) and only discard it when the things it relies on change, or

2. Make it a method (explicitly).

(Or I might do both.)

I expect method calls to incur more overhead than property accesses. I don't know if there's a specific OOP principle one can cite about that, but that's my view from experience (for what it may or may not be worth).

But, note that there are counterexamples out in the wild. A famous and directly applicable one is the innerHTML property of DOM elements, which is an accessor property that does a fair bit of work when you use it.

Here's an example of returning an immutable thing you keep and re-return, but invalidate when its underlying data changes:

class Container {
    constructor() {
        this._content = [];
        this._exposedContent = null;
    }

    addContent(content) {
        this._content.push(content);
        this._exposedContent = null; // Invalidating the cached copy if any
    }

    get content() {
        // Do we have a cached copy?
        if (!this._exposedContent) {
            // No, create it
            this._exposedContent = this._content.map(
                c => Object.freeze(new ContentTypes[c.type](c))
            );
        }
        return this._exposedContent;
    }
}

(You might also expire the copy after a period of time.)

But it's a judgement call whether it's worth the memory overhead and complexity vs. what your code actually in your content accessor. My default position would be to use a method instead (getContent or similar) and not cache, but it's a judgement call.