optimize and fix databinding.deltaFromChange

optimize and fix databinding.deltaFromChange

This is a grabbag of three small changes.  The biggest change is that I did some optimizations of databinding.deltaFromChange and fixed an edge case (with test).  I also optimized _setupDependencies for that edge case, added some documentation about bindings (so I could refer to it myself), and optimized the bind/_bind pair a bit by removing redundant calls to code that iterates over all bindings (they are now done once in bind, rather than once per viewlet in _bind).

https://code.launchpad.net/~gary/juju-gui/fixDeltaFromChangeEdgeCase/+merge/185053

(do not edit description out of merge proposal)

[gary.poster, 2013-09-11 13:37:14]

Please take a look.

[gary.poster, 2013-09-11 13:44:34]

Line-by-line comments for reviewers.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js
File app/views/databinding.js (right):

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:186: var index = indexBindings(bindings, null, true);
The "true" argument and then the "result.bindings.push.apply" below is the real
fix.  All the other changes streamline the logic, resulting in much fewer
duplicate results and hopefully clearer reading.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:393: this._modelChangeHandler();
These were moved from _bind, mostly as an optimization for reading but also to
prevent unnecessary duplication of work.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:533: index[dep] = source;
This is just an optimization, so that if we have more than one of these implicit
bindings on the same source, we don't create multiple copies of the source.  It
wasn't causing a bug, but this just makes it easier to debug further downstream.

[benji, 2013-09-11 13:53:08]

LGTM

I asked for some more words in a couple of spots.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js
File app/views/databinding.js (right):

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:302: * get(model):
It took me a minute to figure out that this was a method with its argument. 
That's probably OK, but it is worth noting.  Also, you probably want something
after the colon.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:308: * update(node, value): (optional)
Maybe the meaning of these methods would be clear if I read more of the code,
but having prose to describe them here would be nice.

[gary.poster, 2013-09-11 14:31:04]

*** Submitted:

optimize and fix databinding.deltaFromChange

This is a grabbag of three small changes.  The biggest change is that I did some
optimizations of databinding.deltaFromChange and fixed an edge case (with test).
 I also optimized _setupDependencies for that edge case, added some
documentation about bindings (so I could refer to it myself), and optimized the
bind/_bind pair a bit by removing redundant calls to code that iterates over all
bindings (they are now done once in bind, rather than once per viewlet in
_bind).

R=benji
CC=
https://codereview.appspot.com/13348056