Get internal ID logic in one place

js/id/connection.js

@@ -67,8 +67,7 @@ iD.Connection = function() {
             delete o.lon;
             delete o.lat;
         }
-        o._id = o.id;
-        o.id = o.type[0] + o.id;
+        o.id = iD.Entity.id.fromOSM(o.type, o.id);
         return iD.Entity(o);
     }
 

js/id/graph/entity.js

@@ -14,8 +14,8 @@ iD.Entity = function(a, b, c) {
         }
     }
 
-    if (!this.id) {
-        this.id = iD.util.id(this.type);
+    if (!this.id && this.type) {
+        this.id = iD.Entity.id(this.type);
         this._updated = true;
     }
 
@@ -28,17 +28,35 @@ iD.Entity = function(a, b, c) {
     }
 };
 
+iD.Entity.id = function (type) {
+    return iD.Entity.id.fromOSM(type, iD.Entity.id.next[type]--);
+};
+
+iD.Entity.id.next = {node: -1, way: -1, relation: -1};
+
+iD.Entity.id.fromOSM = function (type, id) {
+    return type[0] + id;
+};
+
+iD.Entity.id.toOSM = function (id) {
+    return +id.slice(1);
+};
+
 iD.Entity.prototype = {
+    osmId: function() {
+        return iD.Entity.id.toOSM(this.id);
+    },
+
     update: function(attrs) {
         return iD.Entity(this, attrs, {_updated: true});
     },
 
     created: function() {
-        return this._updated && +this.id.slice(1) < 0;
+        return this._updated && this.osmId() < 0;
     },
 
     modified: function() {
-        return this._updated && +this.id.slice(1) > 0;
+        return this._updated && this.osmId() > 0;
     },
 
     intersects: function(extent, resolver) {

js/id/renderer/map.js

@@ -129,7 +129,7 @@ iD.Map = function() {
             .filter(filter)
             .data(waynodes, key);
         function olderOnTop(a, b) {
-            return (+a.id.slice(1)) - (+b.id.slice(1));
+            return a.osmId() - b.osmId();
         }
         handles.exit().remove();
         handles.enter().append('image')

js/id/ui/inspector.js

@@ -10,7 +10,7 @@ iD.Inspector = function() {
             .attr('class', 'permalink')
             .attr('href', function(d) {
                 return 'http://www.openstreetmap.org/browse/' +
-                d.type + '/' + d.id.slice(1);
+                d.type + '/' + d.osmId();
             })
             .text('View on OSM');
         selection.append('a')

js/id/util.js

@@ -1,13 +1,5 @@
 iD.util = {};
 
-iD.util._counters = {};
-
-iD.util.id = function(counter) {
-    counter = counter || 'default';
-    if (!iD.util._counters[counter]) iD.util._counters[counter] = 0;
-    return counter[0] + (--iD.util._counters[counter]);
-};
-
 iD.util.trueObj = function(arr) {
     var o = {};
     for (var i = 0, l = arr.length; i < l; i++) o[arr[i]] = true;

test/spec/graph/entity.js

@@ -1,4 +1,4 @@
-describe('Entity', function () {
+describe('iD.Entity', function () {
     if (iD.debug) {
         it("is frozen", function () {
             expect(Object.isFrozen(iD.Entity())).to.be.true;
@@ -13,6 +13,24 @@ describe('Entity', function () {
         });
     }
 
+    describe(".id", function () {
+        it("generates unique IDs", function () {
+            expect(iD.Entity.id('node')).not.to.equal(iD.Entity.id('node'));
+        });
+
+        describe(".fromOSM", function () {
+            it("returns a ID string unique across entity types", function () {
+                expect(iD.Entity.id.fromOSM('node', 1)).to.equal("n1");
+            });
+        });
+
+        describe(".toOSM", function () {
+            it("reverses fromOSM", function () {
+                expect(iD.Entity.id.toOSM(iD.Entity.id.fromOSM('node', 1))).to.equal(1);
+            });
+        });
+    });
+
     describe("#update", function () {
         it("returns a new Entity", function () {
             var a = iD.Entity(),
@@ -105,7 +123,7 @@ describe('Entity', function () {
     });
 });
 
-describe('Node', function () {
+describe('iD.Node', function () {
     it("returns a node", function () {
         expect(iD.Node().type).to.equal("node");
     });
@@ -138,7 +156,7 @@ describe('Node', function () {
     });
 });
 
-describe('Way', function () {
+describe('iD.Way', function () {
     if (iD.debug) {
         it("freezes nodes", function () {
             expect(Object.isFrozen(iD.Way().nodes)).to.be.true;
@@ -191,7 +209,7 @@ describe('Way', function () {
     });
 });
 
-describe('Relation', function () {
+describe('iD.Relation', function () {
     if (iD.debug) {
         it("freezes nodes", function () {
             expect(Object.isFrozen(iD.Relation().members)).to.be.true;

test/spec/util.js

@@ -1,16 +1,6 @@
 describe('Util', function() {
     var util;
 
-    it('#id', function() {
-        var a = iD.util.id(),
-        b = iD.util.id(),
-        c = iD.util.id(),
-        d = iD.util.id();
-        expect(a === b).to.equal(false);
-        expect(b === c).to.equal(false);
-        expect(c === d).to.equal(false);
-    });
-
     it('#trueObj', function() {
         expect(iD.util.trueObj(['a', 'b', 'c'])).to.eql({ a: true, b: true, c: true });
         expect(iD.util.trueObj([])).to.eql({});