Code review comment for lp:~makyo/juju-gui/endpoints-once

Reviewers: mp+137273_code.launchpad.net,

Message:
Please take a look.

Description:
Clean up get_endpoints

get_endpoints was being called on every instance of service_add and not
being garbage collected. Moved it to on_database_changed and added gc
around services being removed.

https://code.launchpad.net/~makyo/juju-gui/endpoints-once/+merge/137273

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6858099/

Affected files:
   A [revision details]
   M app/app.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/app.js
=== modified file 'app/app.js'
--- app/app.js 2012-11-20 16:55:43 +0000
+++ app/app.js 2012-11-30 17:33:27 +0000
@@ -256,8 +256,6 @@
        // TODO - Bound views will automatically update this on individual
models
        this.db.on('update', this.on_database_changed, this);

- this.db.services.on('add', this.updateEndpoints, this);
-
        this.on('navigate', function(e) {
          console.log('app navigate', e);
        });
@@ -310,6 +308,35 @@
       */
      on_database_changed: function(evt) {
        Y.log(evt, 'debug', 'App: Database changed');
+
+ var updateNeeded = false,
+ self = this;
+ if (!Y.Lang.isValue(this.serviceEndpoints)) {
+ this.serviceEndpoints = {};
+ }
+
+ // Compare endpoints map against db to see if it needs to be changed.
+ this.db.services.some(function(service) {
+ if (self.serviceEndpoints[service.get('id')] === undefined) {
+ updateNeeded = true;
+ }
+ return updateNeeded;
+ });
+
+ // If there are new services in the DB, pull an updated endpoints
map.
+ if (updateNeeded) {
+ this.updateEndpoints();
+ } else {
+ // Check to see if any services have been removed (if there are,
and
+ // new ones also, updateEndpoints will replace the whole map, so
only
+ // do this if needed).
+ Y.Object.each(this.serviceEndpoints, function(key, value, obj) {
+ if (self.db.services.getById(key) === null) {
+ delete(self.serviceEndpoints[key]);
+ }
+ });
+ }
+
        // Redispatch to current view to update.
        this.dispatch();
      },
@@ -322,9 +349,6 @@
      updateEndpoints: function(callback) {
        var self = this;

- if (!Y.Lang.isValue(this.serviceEndpoints)) {
- this.serviceEndpoints = {};
- }
        // Defensive code to aid tests. Other systems
        // don't have to mock enough to get_endpoints below.
        if (!this.env.get('connected')) {