CODEWITHSUNDEEP
javascript
Lukich
Lukich

Reputation: 716

Is it a bad practice to add reference to a Javascript object in one of its attributes?

Let's say I have a setup like this.

var Account = function(data) {
  this.data = data;
  this.domElement = (function(){ code that generates DOM element that will represent this account })();
  this.domElement.objectReference = this;
}

Account.prototype = {
  show: function() { this.domElement.classList.remove('hidden'); },
  hide: function() { this.domElement.classList.add('hidden') }
}

My question is about the last line: this.domElement.objectReference = this;

It would be a useful thing to have because then I can add event listeners to my DOM element and still get access to the object itself. I can add methods that would affect my DOM element, such as hide() and show(), for instance, without having to resort to modifying visibility of the DOM element using CSS directly.

I tested this code and it works like I want it to, but I'm curious whether this would cause memory leaks or some other unpleasantness or if it's an acceptable thing to do?

Thank you! Luka

Upvotes: 0

Views: 110

Answers (3)

plalx
plalx

Reputation: 43718

I know this has been answered by @PaulS. already, but I find the answer counter intuitive (returning a DOM element from the Account constructor is not expected) and too DOM-centric, but at the same time the implementation is very simple, so I am not sure what to think ;)

Anyway, I just wanted to show a different way of doing it. You can store Account instances in a map and give them a unique id (perhaps they have one already), then you store that id as a data attribute on the DOM element. Finally you implement a getById function or something similar to retrieve the account instance by id from the listeners.

That's pretty much how jQuery's data works.

Here's an example with delegated events like you wanted from the comments.

DEMO

var Account = (function (accounts, id) {

    function Account(data) {
        accounts[this._id = ++id] = this;

        this.el = createEl.call(this);
    }

    Account.prototype = {
        constructor: Account,
        show: function() { this.el.classList.remove('hidden'); },
        hide: function() { this.el.classList.add('hidden'); }
    };

    function createEl() {
        var el = this.el = document.createElement('div');
        el.className = 'account';
        el.innerHTML = el.dataset.accountId = this._id;

        return el;
    }

    Account.getById = function (id) {
        return accounts[id];
    };

    Account.init = function () {
        //add delegate listeners

        document.addEventListener('click', function (e) {
            var target = e.target,
                account = Account.getById(target.dataset.accountId);

            if (!account) return;

            account.hide();
        });
    };

    return Account;
})({}, 0);

//once DOM loaded
Account.init(); //start listening to events

var body = document.body;

body.appendChild(new Account().el);
body.appendChild(new Account().el);

Upvotes: 2

Paul S.
Paul S.

Reputation: 66334

Why not have domElement as a variable, and return it from your function? To keep the reference to your constructed Object (but only where this is as expected), you could do a if (this instanceof Account) domElement.objectReference = this;

You've now saved yourself from circular references and can access both the Node and the Object. Doing it this way around is more helpful if you're expecting to lose the direct reference to your Account instance, but expect to need it when "looking up" the Node it relates to at some later time.

Code as requested

var Account = function (data) {
    var domElement; // var it
    this.data = data;
    domElement = (function(){/* ... */}()); // use var
    if (this instanceof Account)
        domElement.objectReference = this; // assign `this`
    return domElement;
};
// prototype as before

Returned element is now the Node, not the Object; so you'd access the Account instance like this

var domElement = new Account();
domElement.objectReference.show(); // for example

Upvotes: 1

Salvador Dali
Salvador Dali

Reputation: 222591

In my opinion there is nothing good about referencing the object inside of the object itself. The main reason for this is complexity and obscurity.

If you would point out how exactly are you using this domElement.objectReference later in the code, I am sure that I or someone else would be able to provide a solution without this reference.

Upvotes: 0

Related Questions