Javascript: Dynamically created functions in a given object issue

Question

While this question have been asked before and many already answered it, my question is strictly about the prototype of the newly created functions.

If you read this piece of code, you will understand that it just works. Also here on codepen.

    // main object
    var Foo = {}; 

    // main methods
    Foo.render = {}; // the Render function to populate later

    Foo.start = function(el,ops){
      return new Actions(el,ops);
    }

    // secondary/utility functions
    var Actions = function(el,ops){
      this.el = document.querySelector(el);
      this.ops = ops || {};
      this.prepare(); // this builds the Foo.render functions
      for (var p in this.ops){
        Foo.render[p](this);
      }
    }; 


    // Action methods
    Actions.prototype.prepare = function(){
      for (var p in this.ops) {
        Foo.render[p] = function(that){ // or r[p]
          that.el.style[p] = that.ops[p] + 'px';
        } 
      }
    }

    // init
    var action = new Foo.start('div',{left:15})

    // check
    console.log(Foo.render['left'].prototype);

<div></div>

The problem is the prototype of the newly created function Foo.render['left'] is something like this Foo.render.(anonymous function) {} instead of something like Foo.render.left() {} or something else, and I am experiencing some performance loss because I am unable to access the newly created function's prototype very fast.

Can anyone please shed some light on how to adapt the .prepare() function to create accurate/accessible (I can't chose the right word) prototype functions within the Foo scope?

Thank you.

All functions do have Function.prototype as their prototoype. There's nothing wrong with that. — Bergi
– Bergi, Commented Feb 2, 2016 at 23:51
You should add an if (!(p in Foo.render)) to that loop so that you're not recreating functions for every new Actions instance. — Bergi
– Bergi, Commented Feb 2, 2016 at 23:53
You seem to have the standard closure in a loop issue, but I can't really see what you want to do here or why. — Bergi
– Bergi, Commented Feb 2, 2016 at 23:54
@Bergi thanks for your valuable comments and link. I'm basically trying to avoid a Foo.render() function flowing 30+ ifs, and replace that with a nice object that only holds all functions that are really required. Less code, more performance modularity/extensibility, and generally a more elegant solution. Please take a look at my answer below, seems to be close to the checked answer you linked, do you approve it or know a better solution considering HTML4 browsers support? — thednp
– thednp, Commented Feb 3, 2016 at 0:18
I see that you're trying to avoid ifs to check check which properties are present. But then, after setting up multiple Foo.render[p] functions, you still go through them in a loop over ops. I wonder why you don't simply create a single render method, and put the loop inside there? Apart from that, the solution in your answer seems fine (as in "working correctly", not necessarily "well designed"). — Bergi
– Bergi, Commented Feb 3, 2016 at 0:25

Bergi · Accepted Answer · 2016-02-03 15:47:06Z

1

You will need to capture the value of p in an extra closure scope. Also I would recommend avoiding to overwrite already-existing methods.

Actions.prototype.prepare = function() {
  for (var p in this.ops) {
    if (!(p in Foo.render)) {
      Foo.render[p] = (function(prop) {
        return function(that) {
          that.el.style[prop] = that.ops[prop] + 'px';
        };
      }(p));
    } 
  }
};

or

Actions.prototype.prepare = function() {
  for (var p in this.ops) {
    (function() {
      var prop = p;
      if (!(prop in Foo.render)) {
        Foo.render[prop] = function(that) {
          that.el.style[prop] = that.ops[prop] + 'px';
        };
      }
    }());
  }
}

edited Feb 3, 2016 at 15:47

answered Feb 3, 2016 at 15:28

Bergi

671k162 gold badges1k silver badges1.5k bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

thednp Over a year ago

In the second version there could be some type errors, but I will check both, so far first seems ok. I'll mark your question anyway since you're so awesome.

thednp Over a year ago

I would need a Foo.render[prop] = Foo.render[prop].prototype or something to improve the access time for these functions, as the left.prototype is still Foo.render.(anonymous function), so it doesn't seem to have a name property.

Bergi Over a year ago

@thednp: That a function has no name property does have zero influence on its speed

thednp Over a year ago

OK now, tested both the solutions you provided in all cases and they both make total sense with everything else you mentioned before, it's mixing up the p without it bound in the context. Thank you so much.

thednp Over a year ago

I updated my answer, please feel free to comment on the new idea.

thednp · Accepted Answer · 2016-02-04 18:21:12Z

0

I think I found a way to make it work better. The following should do, however I'm still curious to know if there is any better solution.

// main object
var Foo = {}; 

// main methods
Foo.render = {}; // the Render function to populate later

Foo.start = function(el,ops){
   return new Actions(el,ops);
}

// secondary/utility functions
var Actions = function(el,ops){
   this.el = document.querySelector(el);
   this.ops = ops || {};
   this.prepare(); // this builds the Foo.render functions
   for (var p in this.ops){
     Foo.render[p](this);
   }
}; 


// Action methods
Actions.prototype.prepare = function(){
   for (var p in this.ops) {
      Foo.render[p] = (function(){ // or r[p]
         return function(that){ 
           that.el.style[p] = that.ops[p] + 'px';
         }
      })(); 
   }
};

// init
var action = new Foo.start('div',{left:15})

// check
console.log(Foo.render['left'].prototype);

UPDATE: I think I found a way to eliminate one of the closures, basically using the p as the function's second attribute, like so Foo.render[p] = function(that,p){} here we go:

// main object
var Foo = {}; 

// main methods
Foo.render = {}; // the Render function to populate later

Foo.start = function(el,ops){
   return new Actions(el,ops);
}

// secondary/utility functions
var Actions = function(el,ops){
   this.el = document.querySelector(el);
   this.ops = ops || {};
   this.prepare(); // this builds the Foo.render functions
   for (var p in this.ops){
     Foo.render[p](this,p); // we include p here
   }
}; 


// Action methods
Actions.prototype.prepare = function(){
   for (var p in this.ops) {
      Foo.render[p] = function(that,p){ // we also include p here
         that.el.style[p] = that.ops[p] + 'px';
      }; 
   }
};

// init
var action = new Foo.start('div',{left:15})

// check
console.log(Foo.render['left'].prototype);

<div></div>

This eliminates the additional closure and brings the function closer to the main thread's scope. Any comment on the update is welcome.

edited Feb 4, 2016 at 18:21

answered Feb 2, 2016 at 23:35

thednp

4,4794 gold badges36 silver badges51 bronze badges

11 Comments

Bergi Over a year ago

I hope you're ok with me editing your answer instead of posting my own; feel free to roll it back if not

thednp Over a year ago

I'm happy to learn, please feel free to play and I thank you :)

thednp Over a year ago

I updated my code based on your changes, binding the p into the (function(p) {})(p) context is not needed, as that is the link between the two main object and it seems to slow the access time a bit.

Bergi Over a year ago

Binding p is the reason why you added the closure in the first place, right? Without it, when you do new Actions(…, {left:…, right:…}) a call to Foo.render.left(…) would update the right property.

thednp Over a year ago

You may be right, but the fact that the returned function body uses the p, it's creating a new Closure object with p: "left" so your idea to bind will create an additional Closure, I'm not sure if this is good or not, I believe not.

|

Collectives™ on Stack Overflow

Javascript: Dynamically created functions in a given object issue

2 Answers 2

5 Comments

11 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

11 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related