Skip to content

Instantly share code, notes, and snippets.

@conradz
Created April 5, 2013 20:16
Show Gist options
  • Save conradz/5322280 to your computer and use it in GitHub Desktop.
Save conradz/5322280 to your computer and use it in GitHub Desktop.
Global Delay State

Since thinking more on the function/delay implementation, I've come to the conclusion that it basically is unusable in many cases when it caches functions globally.

Simple Example

function Item(el) {
  this.el = el;
}

Item.prototype.resize = function(width, height) {
  $(this.el).width(width).height(height);
}

function Application() {
  this.nav = new Item($(".nav"));
  this.body = new Item($(".body"));
}

Application.prototype.resize = function() {
  delay(this.nav.resize, 200, this.nav, 150, 500);
  delay(this.body.resize, 300, this.body, 500, 500);
}

var app = new Application();
app.resize();

Can you guess what the result is? The .nav element is unchanged, but the .body element is changed. This happens because this.nav.resize === this.body.resize, since it gets .resize from Item.prototype. The two delay calls needn't be together, they could be in entirely different parts of the app, on different objects, but as long it gets the same function from the prototype with 200ms it will not work. This IMO makes it way too easy to mess up with it. Functions should be designed to be easy to use, while making it harder to write obviously incorrect code.

Functional Example

The solution to the first example would be to cache the context property also, but this breaks down if not using an OO approach, as in the following:

function resize(el, width, height) {
  $(el).width(width).height(height);
}

function Application() {
  this.nav = $(".nav");
  this.body = $(".body");
}

Application.prototype.resize = function() {
  delay(resize, 200, null, this.nav, 150, 500);
  delay(resize, 300, null, this.body, 500, 500);
}

var app = new Application();
app.resize();

While this example is more obvious when considering overrides, it still is something that could happen, especially when using a function from another module (e.g. resize could be a complex function from a separate module). It is not that hard to imagine something that causes this, and the time to track down a bug like this can be extremely hard (timing bugs are some of the worst to track down). Imagine if you were doing an animation that called delay to wait for 10ms before moving it a little. If you had a condition like this the first animation could move at least one step, maybe more, but when the second animation started it could stop the first half-way through the animation. It would be extremely unlikely that you will immediately know that the cause of the animation getting stopped was because you were using delay to call a function on the prototype and that it was getting overriden.

Locally Cached Example:

IMO the cost of debugging and the ease of incorrect use outweighs the slightly more code of caching it locally:

function Item(el) {
  this.el = el;
}

Item.prototype.resize = function(width, height) {
  $(this.el).width(width).height(height);
}

function Application() {
  this.nav = new Item($(".nav"));
  this.body = new Item($(".body"));
  
  var resizeNav = delay(this.nav.resize, this.nav);
  var resizeBody = delay(this.body.resize, this.body);
  this.resize = function() {
    resizeNav(200, 150, 500);
    resizeBody(300, 500, 500);
  }
}

var app = new Application();
app.resize();
@conradz
Copy link
Author

conradz commented Apr 5, 2013

This is just pointing out some flaws that I see in the implementation. Feel free to poke holes in my examples and otherwise discuss pros and cons of the different proposals.

@roboshoes
Copy link

First of all - I'd like to appriciate the amount of thought you put into this and you raise a very valid point. And in matter of fact. There will be lot's of times where I would animate something in a View class. I would constently override my delays as I have multiple instances of the same class.

Since your proposed syntax provides a solution I am very much for it. If anything you can easily appreviate it by writing:

delay( this.body.resize, this.body )( 200 );

This would also make it a one liner and also looks funky - which keeps your code fresh and exciting.

But, seriously, I like it. 👍

@millermedeiros what do you think?

@conradz
Copy link
Author

conradz commented Apr 8, 2013

I had thought previously that I could live with the delay(func, timeout, context, args...) signature and semantics, but the prototype function was the real killer that would have made it very easy to use incorrectly.

If you are using it like delay(func, context)(200), you are much better off just using the function/timeout module proposed in this comment: timeout(func, time, context, args...). It is a common enough use case that I think we should have it.

delay would be used only for a sequence of delays that should not have more than one waiting at any given time. Not sure that delay is a good name for it, though.

@roboshoes
Copy link

It's been a while, my bad (Apperently I do not get email notifications for this gist :/)

I agree with you, as I have used the module I proposed, and now also have to agree (even though it works for me in most cases) that I would be against adding it to mout, for reasons that you have explained in this gist.

However, like you also just said, I would be very pleased to see timeout( func, time, context, args ... ) to go into mout. It was the very original PR and the most used use-case I think.

@roboshoes
Copy link

I'm going to update my Pull Request, as github is not keen on partially accepting a Pull Request (if I'm not mistaking)

@conradz
Copy link
Author

conradz commented Apr 25, 2013

Just saw your comments, I don't get email notifications on the gist either :|

Yep, agree with getting timeout in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment