Method Rewriting: Running With A Lit Stick Of Dynamite

I had a problem I wanted to solve. I thought to myself, “I know! I’ll use function rewriting!” and it was good… I solved the problem. But I also introduced some other problems regarding method references and event handlers. 

Method Rewriting

If you’re not familiar with method rewriting in JavaScript, it’s basically just replacing a method at runtime, from within the method itself.

var myObj = {
 
  foo: function(){
    console.log("bar");
    
    this.foo = function(){
      console.log("quux");
    }
    
  }
 
};

myObj.foo(); // => "bar"
myObj.foo(); // => "quux"
myObj.foo(); // => "quux"
// ...

In this example, the “foo” method contains code to replace the “foo” method with a new method implementation. When the “foo” method is run the first time, it outputs “bar”. Then it replaces the “foo” method with the new implementation. Subsequent execution of this method will output “quux” instead of “bar” because the method has been rewritten.

And BOOM Goes The Dynamite

The problem I ran into has to do with the way we set up event handlers in JavaScript, by passing function references around. Consider the following Backbone.View implementation:

var MyView = Backbone.View.extend({
  
  initialize: function(){
    this.model.on("change", this.render, this);
  }
  
  doSomeExtraWork: function(){
    console.log("extra work");
  },
  
  render: function(){
    this.doSomeExtraWork();
    this.$el.html("... rendered HTML stuff goes here ...");
    
    this.render = function(){
      this.$el.html("... rendered HTML stuff goes here ...");      
    }
  }
  
});

var model = new Backbone.Model();
var view = new MyView({model: model});

view.render(); // => console.log of "extra work"
view.render(); // => no console.log! yay!

model.set("foo", "bar"); // => console.log of "extra work". BOOO!

When the view is instantiated, a “change” event from the view’s model is handled. When the model changes, it calls the “render” function. This ensures the view is always up to date with the latest data from the model. It’s not a very good solution from a performance perspective, but it solves the general problem.

Now let’s introduce a more complicated scenario where the first time the view renders, it should do a little bit of extra work. Subsequent execution of the render method should not do that extra work. I’m not going to get in to detail about what the extra work might be in a real application, but I can illustrate the point using some simple console.log calls. 

When you run the above code, the first call to the render method logs the “extra work” message. Calling the render method a second time, directly, does not produce this message. That’s good. It means our function re-writing worked. However, changing any of the data on the model will produce the “extra work” message, which is not what we want.  Worse still, it will replace the “render” method on the view instance again, and again, and again, every time the change event fires.

Lighting The Dynamite

The problem is the way we hand off a reference to the function for the event handler. When we call `this.model.on(“change”, this.render, this);`, we are handing a method pointer to the “on” function –  a callback that points directly to the current implementation of the “render” function. This is not a reference that gets evaluated every time the change method fires. It is evaluated immediately when the event handler is set up, and a direct reference to the render function itself (not the object that holds on to the render function) is passed through as the event handler.

When the render function is called later on, the render function itself replaces the view’s “render” function with a new function. It re-writes itself… except this is a bit of a misnomer. The function it not actually re-writing itself. What it is doing, really, is replacing the view’s reference to itself with a reference to another function. It’s the equivalent of this:

// set up the initial object and method
var myView = {
  render: function(){ ... }
};

// replace the render method
myView.render = function(){
  // ...
};

In this code, it is very clear that the “render” method is being replaced. The original function is not being “re-written”, but only replaced. Method re-writing does exactly this, but does it from within the function that is being replaced. 

Now, getting back to the event handler… when the “render” function replaces itself, it only replaces the function on the view instance. It doesn’t have a reference to the “on” method and it doesn’t know how to replace that event handler with the new function reference. Therefore, the “on” event handler still references the original render function, even after the render function has replaced itself on the view.

The result is that the “change” event fired from the model will cause the original render function to be executed. Since the original render function contains the method re-writing code, it will replace the view’s render method again. Every time the “change” event fires from the model, this will happen. If the “change” event fires 3 times, the “render” function on the view will be replaced 3 times.

Powerful But Dangerous

Lesson learned: method rewriting is like running with a lit stick of dynamite. It’s a powerful tool that serves a purpose – but the scope and references to the functions that are being re-written need to be controlled very tightly, or it will blow up in your face.


Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Derick Bailey

Derick Bailey is an entrepreneur, problem solver (and creator? :P ), software developer, screecaster, writer, blogger, speaker and technology leader in central Texas (north of Austin). He runs SignalLeaf.com - the amazingly awesome podcast audio hosting service that everyone should be using, and WatchMeCode.net where he throws down the JavaScript gauntlets to get you up to speed. He has been a professional software developer since the late 90's, and has been writing code since the late 80's. Find me on twitter: @derickbailey, @mutedsolutions, @backbonejsclass Find me on the web: SignalLeaf, WatchMeCode, Kendo UI blog, MarionetteJS, My Github profile, On Google+.
This entry was posted in AntiPatterns, Backbone, Javascript, Principles and Patterns. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://ifandelse.com Jim Cowart

    I agree with you 100% here – it’s amazing the kind of flexibility we get with the language, but it really can be lit dynamite. Did you run into this while working on Transistor?

    • http://mutedsolutions.com Derick Bailey

      yeah, Transistor… it works, it forces people to use a different style of event binding, which is a big no-no in playing nice with the community :P back to the drawing board for Transistor… for the 4th time :D

  • http://twitter.com/Matt_Polichette Matt Polichette

    The other place you can shoot yourself in the foot with this problem is with things like the underscore function tools (http://underscorejs.org/#functions).

    I was using underscore to bind a function’s context with bindAll, which I found out (the hard way) will replace the functions, so, if events have already been bound to the original functions… the originals will get called instead… :-/ Bind before!

  • leogcrespo

    For safety, you could just do method rewriting with “private” methods, which you never listen directly. It adds some indirection, for example in the render function by calling some _internalRender or something like that, but if that is what you really need, it is a safer path.

  • petehawkins

    I’ve often wanted to replace methods with spys in Backbone for testing purposes, replace render with a spy, though ran into this issue, would love to optionally pass a string of the method name rather than a pointer to the method, then it simply does something like:

    this[methodName].call(this);

    Would alleviate these problems in Backbone, though I have no idea of the performance issues with this approach.

    Instead with certain tests I’m altering the prototype of a view, replacing it with a spy and then setting it back in the teardown, don’t like this approach much, want to avoid editing the actual constructors prototype in a test, editing one instance of an object would be much nicer.

  • http://twitter.com/liddellj Jim Liddell

    What about attaching the change event to render using Backbone’s ‘once’ instead of ‘on’ within initialize, and then doing the same again within the re-written implementation of render each time it’s called? I can’t think of any logical disadvantage, but I’m not sure if it’s bad practice or would have a performance hit. Thoughts?

    • http://mutedsolutions.com Derick Bailey

      that would probably work… but it would still require anyone using the method rewriting to write their event handlers differently than they normally would. that’s really the issue, here. i don’t want to force people to change how they bind to events, just to work around a pattern that i decided to use :)