imkost
imkost

Reputation: 8163

Is it ok to dynamically change function?

I have:

var diagram = {
  init: function(name) {
    this._name = name;
  },

  update: function(params) {
    var selector = 'diagram_name_' + this._name;

    this.update = function(params) {
      app.updateBlock(selector, params);
    };

    this.updateDiagram(params);
  };

diagram.update(); // caches `selector`
diagram.update(); // uses cached `selector`

diagram object has method update. This method uses selector variable. First time calling update(), I cache this variable and change update method so next time I call it, this method will use cached selector.

I would like to know if this approach is ok in terms of performance (and if it is ok at all). Any ideas?

P.S.

I know I can create private field _selector and execute this._selector = 'diagram_name_' + name during init method, but this approach creates private field that is used by one method only, does not feel right.

Upvotes: 1

Views: 71

Answers (3)

Phil Jackson
Phil Jackson

Reputation: 456

A better way might be to cache selector on the object:

this._selector || (this._selector = "value");

That way, there's less magic and you still have access to all of the old data instead of making it disappear.

What you also might want to checkout is the technique known as memoization, something that's been solved many times (to avoid having to do it yourself).

Upvotes: 1

basilikum
basilikum

Reputation: 10526

There is no need to worry about performance and the update method will also behave in the expected way. So technically, this pattern will work.

However, I find it rather confusing. Of course, when it comes to code clarity, things are always a bit subjective, but here is what I would suggest:

Method 1

Separate the two cases completely and make sure that outside code calls updateDiagram before it calls update:

var diagram = {
    init: function(name) {
        this._name = name;
    },

    update: function(params) {
        var selector = 'diagram_name_' + this._name;
        app.updateBlock(selector, params);
    },

    updateDiagram: function(params) {
        //...
    }
};

Method 2

Use parameter to indicate which of the cases should be used:

var diagram = {
    init: function(name) {
        this._name = name;
        this._diagramUpToDate = false;
    },

    update: function(params) {
        if (this._diagramUpToDate) {
            var selector = 'diagram_name_' + this._name;
            app.updateBlock(selector, params);
        } else {
            this.updateDiagram(params);
        }
    },

    updateDiagram: function(params) {
        //...
        this.diagramUpToDate = true;
    }
};

Of course it is your choice what you use in the end. As I said, your approach also works fine and the only issue might be code clarity.

Upvotes: 0

Alex Soroka
Alex Soroka

Reputation: 810

Yes it's ok. It's called Self-Defining Functions. Also the pattern was described in the awesome book "JavaScript Patterns" by Stoyan Stefanov.

Upvotes: 0

Related Questions