0

I have been exploring patterns in various MV* frameworks out there and today noticed a weird one, which seems to cause some issues

Model prototype. has a property collections: []

Collection prototype. has a property models: []

When a collection gets a new model, it is being pushed into collection.models but the model itself is also decorated to be aware of the collection it is a member of - i.e. the collection instance is pushed into model.collections.

so model.collections[0] is a collection that contains a .models[0] being the model that has a collection property... and so on.

at its most basic:

var A = function() {
    this.collections = [];
},
    B = function() {
        this.models = [];
        this.add = function(what) {
            what.collections.push(this);
            this.models.push(what)
        };
    };


var model = new A();
var collection = new B();

collection.add(model);

Here's the guilty party in action: https://github.com/lyonbros/composer.js/blob/master/composer.js#L310-313 and then further down it's pushing into models here: https://github.com/lyonbros/composer.js/blob/master/composer.js#L781-784

I suppose there is going to be a degree of lazy evaluation - things won't be used until they are needed. That code - on its own - works.

But I was also writing tests via buster.js and I noticed that all the tests that had reliance on sinon.spy() were producing InternalError: too much recursion (FF) or RangeError: Maximum call stack size exceeded(Chrome). The captured FF was even crashing unresponsively, which I have never encountered with buster test driver before - it even went to 3.5gb of ram use over my lunch break.

After a fair amount of debugging, I undid the reference storage and suddenly, it was all working fine again. Admittedly, the removal of the spy() assertions also worked but that's not the point.

So, the question is - having code like that, is it acceptable, how will the browsers interpret it, what is the bottleneck and how would you decorate your models with a pointer to the collection they belong in (perhaps a collection controller and collection uids or something).

full gist of the buster.js test that will fail: https://gist.github.com/2960549

Dimitar Christoff
  • 26,147
  • 8
  • 50
  • 69

2 Answers2

2

The browsers don't care. The issue is that the tool you were using failed to check for cyclic reference chains through the object graph. Those are perfectly legitimate, at least they are if you want them and expect them.

If you think of an object and its properties, and the objects referenced directly or indirectly via those properties, then that assembly makes up a graph. If it's possible to follow references around and wind up back where you started, then that means the graph has a cycle. It's definitely a good thing that the language allows cycles. Whether it's appropriate in a given system is up to the relevant code.

Thus, for example, a recursive function that traverses an object graph without checking to see if it's already visited an object will definitely trigger a "too much recursion" error if the graph is cyclic.

Pointy
  • 405,095
  • 59
  • 585
  • 614
  • I am not using a tool as such - buster.js is a testing framework. it does assertions. a `spy` is simpy a function that gets called. it works like jstestdriver and captures devices and uses them as hosts to run the tests. http://busterjs.org/ – Dimitar Christoff Jun 20 '12 at 19:29
  • That's the tool I'm talking about. – Pointy Jun 20 '12 at 19:31
  • so how can an assertion tool look for references? all it does is, `spy = this.spy(); spy(); assert.called(spy);` - it does not in any way touch the code or need to know about it - all it cares for is that it is being run... – Dimitar Christoff Jun 20 '12 at 19:32
  • I don't know exactly what it's doing, but it's clear that somewhere inside that code there's something that doesn't handle the case of an object graph with cycles. Maybe that means the test is inappropriate; I can't really say. – Pointy Jun 20 '12 at 19:41
  • ah I see. Ok - thank you, I will wait for the buster.js people to check out their spies as well but I will accept that a circular reference should not be a problem. what about footprint for memory/gc etc? – Dimitar Christoff Jun 20 '12 at 19:43
  • 1
    circular references have no larger memory footpring than usual ones. Every garbage collector will collect them. The spy() function comes from [sinon.js](https://github.com/cjohansen/Sinon.JS/blob/master/lib/sinon/spy.js), not from the buster.js people. – Bergi Jun 20 '12 at 19:45
  • I *think* most JavaScript garbage collectors are "mark and sweep", but I'm not that educated on the subject. (If they are mark and sweep, or something like that instead of simple reference counting, then there shouldn't be a problem, and I certainly don't know of any "folk wisdom" about circular references being a problem.) – Pointy Jun 20 '12 at 19:47
  • 1
    See [Circular references in Javascript / Garbage collector](http://stackoverflow.com/q/7347203/1048572) for that... – Bergi Jun 20 '12 at 20:01
1

There will only be two objects referencing each other (called "circular reference").

var a, b = {a: a={b: b}};
// a.b: pointer to b
// b.a: pointer to a

There is no recursion at all. If you are getting too much recursion or Maximum call stack size exceeded errors, there needs to be a function which is invoked too often. This could e.g. happen when you try to clone the objects and recurse over the properties without caring for circular references. You'll need to look further in your code, also the error messages should include a (very long) call stack.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375