6

I'm trying to take an existing working code base and make it object oriented using JavaScript. My system takes JSON containing groups and items in a one-to-many relationship, and visualizes this on a page. The items can be moved into different groups, and their positioning within those groups also needs to be calculated. As such, events will need to be established which are aware of both the groups and tickets around them.

I'm using John Resig's simple JavaScript inheritence setup to establish two classes, Item and Group. When each Item is instantiated it refers back to it's parent Group. My issue arises when I want to establish my events, and is most easily explained by the following function:

var Group = Class.extend({
  ...
  // Calculate where to place the new item within the group
  calculate_new_position: function(item) {
    var pos = -1;
    // Loop through all DOM items in groups body
    $(".item", this.elBody).each(function(i) {

      // Retrieve it's class object
      var next = $(this).data("_obj");

      // Calculating things using the class reference
      var lowerPrio = item.tData.priority < next.tData.priority,
          lowerId =  item.id < next.id;

      if(lowerPrio || lowerId) {
        pos = i;
        return false;
      }
    });
    return pos;
  },
  ...
)};

Note the use of the .data("_obj") in the above snippet. Essentially, when I need to do something like sorting the items, then I need to know the object (model) corresponding to every item's DOM (view/controller) in the group. Now, I could establish my Group class so that when I create each Item I add a reference to it from within my Group (e.g. so Group.items = [i1, i2...]), and then rather than iterating over the DOM elements, I could iterate over the Item instances. However I think I'd run into similar problems down the line, such as when I want to move an Item into a different Group (as the Group would not know about the Item).

Long story short: is it intrinsically dangerous/naive/pointless to have a class which when instantiated creates a DOM element, which then in turn points back to the class? This feels like circular dependency, and some recursive nightmare, but perhaps a reference to an object isn't such a terrible thing. If I'm doing anything else really stupid, and there's a much simpler way, then please point that out as well.

tereško
  • 58,060
  • 25
  • 98
  • 150
Ian Clark
  • 9,237
  • 4
  • 32
  • 49
  • Have you had a look at how Backbone.js does it? A `view` object holds reference to the `model`(your Item, Group class) and the DOM element `el`. There are multiple ways of doing it and Backbone.js does not recommend any one way here: http://backbonejs.org/#FAQ-tim-toady – vsr Aug 09 '13 at 11:26

3 Answers3

2

Any modern browser garbage collector can handle circular references. Your object will be garbage collected if you both lose all references to the object and remove all the HTML nodes from the DOM (while its in the DOM the browser will see the reference to your object and prevent it from being garbage collected). Be careful with event handlers though, they can keep refences if you don't remove them as well.

I heard some older versions of IE do have problem with circular references. For more in depht explanation: Precise explanation of JavaScript <-> DOM circular reference issue

From that answer: jQuery's .data() makes thing more reliable because the older versions of IE had a particular problem with properties that were added to a DOM element and did not handle circular references properly involving the data in those properties and thus would not free things when it should have (a leak). .data() works around that by only using one added property on the DOM element which is a safe, non-leaking string. That string is then a key into a javascript object that can contain all the properties that you would like to associate with the DOM element. Because these properties are all store in plain javascript, where the browsers don't have the circular reference bugs, doing it this way doesn't cause leaks.

You might want to look into D3.js it does something similar to that you want. It binds data to DOM elements and provides easy way to generate visualizations: http://d3js.org/

Using D3 you can bind an array of numbers to an array of circle SVG tags and make it so that each circle has a radius based on the number of the of the array its associated with.

EDIT: I forgot to mention that if you use $.remove() the event handlers are removed as well. But if you have for example an event handler that is inside a closure that also has a reference to your (removed) HTML node it will not be garbage collected. This is usually not a big problem because once it's outside the DOM it will not be consuming too much resources and it's not possible to recurservely stack those closure references as far as I know.

Community
  • 1
  • 1
Hoffmann
  • 14,369
  • 16
  • 76
  • 91
1

When using jQuery.data it doesn't do anything with the dom element. The dom element simply stores a primitive key entry into jQuery internal cache, regardless*. The reference you create therefore is from JS to JS:

var div = document.createElement("div");
$(div).data("asd", "daa")
console.log(div[jQuery.expando]);
//41
console.log(jQuery.cache[41].data.asd);
//"daa"

(obviously the above is jQuery internals and should not be relied on in production code)

That said, the data will leak there if you don't go through jQuery to do all your manipulations because jQuery won't be notified if you go behind its back.

Best practice with widget classes is to provide a destroy method that removes all elements it is responsible for, unbinds any events it attached and sets all its DOM references to null.

*If there is absolutely no events or data about the element, then an entry doesn't exist. But if an entry exists for any reason, then using jQuery.data will not interact with the element itself.

Esailija
  • 138,174
  • 23
  • 272
  • 326
1

@Hoffmann's answer is good as he's offering practical advice, with explanations. I offer a different perspective, inspired by how Backbone.js works. This is an expansion on what @vsr suggested in their initial comment.

The basic thrust: Don't store your JS objects in $.data(), at all.

Using $.data() suggests you use $(foo) to look up DOM objects, then drill down into their properties to get to your custom JS functions, which might operate on more DOM elements to get at their custom functions. This is overly complicated and inefficient.

A better approach is to create a set of "pure JS" objects, using whatever syntax and mechanism suits your needs. These objects can then have a property that points to their DOM element for those few occasions where it's needed. Backbone (again) suggests it good practice to also cache the jQuery selected DOM element, as well. Processing these "pure JS" objects will be faster and easier to maintain than going via $.data(). It also increases the abstraction between your JS and HTML, which is a Good Thing.

Further, I've found a good way to work is NOT to nest your objects, even when there's only every a one-to-many relationship. Don't make "items" properties of "groups". Instead, maintain arrays of both groups and items and have one (or both) manage the association by a reference rather than membership.

Simon Dell
  • 638
  • 1
  • 7
  • 17