Neo4J Rest Javascript wrapper

Question

As a toy project, I started evaluating the Neo4J graph database and its Rest interface. I'm trying to write a simple graph visualization in JavaScript. In daily business I'm a Java developer and maybe that's the reason why I'm not that happy with my current approach. In Java I would have written a Neo4J class, with inner (data) classes for Node and Relation as their id is tightly couple to the server (http://localhost:7474/db/data/node/0). Maybe they even would get a common parent class.

In JavaScript I don't get away from my object oriented mind. I need some ideas in relation to JavaScript best practice.

function Neo4J(server) {
 this.server = server;
}
function Node(connection, response) {
 this.id = connection.sanitizeId(response['self']);
 this.data = response['data'];
}
function Relation(connection, response) {
 this.id = connection.sanitizeId(response['self']);
 this.start = connection.sanitizeId(response['start']);
 this.end = connection.sanitizeId(response['end']);
 this.type = response['type'];
}
Neo4J.prototype = {
 sanitizeId: function (id) {
 return id.replace(this.server, "");
 },
 query: function (query, success) {
 return post(this.server + "/cypher", {"query": query, "params": {} }, success);
 },
 getAllNodes: function (callback) {
 var connection = this;
 this.query("MATCH n RETURN n", function (nodes) {
 $.each(nodes['data'], function (i, val) {
 callback(new Node(connection, val[0]));
 });
 });
 },
 getAllRelations: function (callback) {
 var connection = this;
 this.query("START r=rel(*) RETURN r", function (nodes) {
 $.each(nodes['data'], function (i, val) {
 callback(new Relation(connection, val[0]));
 });
 });
 },
 getNode: function (id, callback) {
 var connection = this;
 get(this.server + id + "/properties", function (data) {
 if (data == null) data = {data: {}};
 else data = {data: data};
 data['self'] = id;
 callback(new Node(connection, data));
 });
 },
 createNode: function (callback) {
 var connection = this;
 post(this.server + '/node', null, function (data) {
 callback(new Node(connection, data));
 });
 },
 setNodeProperty: function (node, key, value, callback) {
 put(this.server + node.id + '/properties/' + key, '"'+value+'"', function (data) {
 node.data[key] = value;
 callback(node);
 });
 },
 createRelation: function (start, target, type, callback) {
 var connection = this;
 post(this.server + start + '/relationships', {to: this.server + target, type: type}, function (data) {
 callback(new Relation(connection, data));
 });
 }
}

The post() and put() are just ajax calls to the Neo4J server.

Answer 1

Interesting question,

The most striking thing is your repeated use of this pattern:

connection.sanitizeId(response['xxx']);

I dont believe sanitizeId should be a function of connection. Also, response is not a good name because it often is part of a response from the query and not the actual response. And, it is suggested that response.xxx makes more sense than response['xxx']. To that end, I feel that sanitizeId should be part of the response or data. Also, I would be tempted to call it simply sanitize.

function addSanitizer( data , server){
 data.sanitize = function( propertyName ){
 return this[propertyName].replace( server , "" );
 }
 return data;
}

Then you could do

function Node(response) {
 this.id = response.sanitize( 'self' );
 this.data = response.data;
}

Or, if you agree that response might be misleading

function Node(node) {
 this.id = node.sanitize( 'self' );
 this.data = node.data; 
}

Node with be called like this:

getNode: function (id, callback) {
 var connection = this;
 get(this.server + id + "/properties", function (data) {
 if (data == null) data = {data: {}};
 else data = {data: data};
 data['self'] = id;
 callback(new Node(this.addSanitizer( data, this.server ), data));
 });
},

From a style point:

• You are skipping new lines on your if statements: if (data == null) data = {data: {}};, dont do that

• Your code stretches quite a bit horizontally, but not too bad, feel free to to split up some of your longer lines into something more readable, this :

  createRelation: function (start, target, type, callback) {
   var connection = this;
   post(this.server + start + '/relationships', {to: this.server + target, type: type}, function (data) {
   callback(new Relation(connection, data));
   });
  }
  

  could be

  createRelation: function (start, target, type, callback) {
   var connection = this,
   data = {to: this.server + target, type: type};
   post(this.server + start + '/relationships', data , function (data){
   callback(new Relation(connection, data));
   });
  }
  

The code looks all in all fine to me. For a Java developer you did good :P

• mheinzerling: Thanks for the feedback. I guess the if is more a religious discussion. If I add a line break, I also have to add braces and so I get 3-4 lines. Also a line length of 160 character is only an issue in web representation. But you are right, a final refactoring is missing anyway.
• mheinzerling: I like the idea of the sanitizer, but have the feeling that this is a kind of over-engineered if I only need it for exactly one field? In my OO-mind I think the Neo4J(Server) class should be responsible for transforming an absolute server specific id to a relative id (and vice versa)? I really need to get used to bind this to the actual data. Btw. the response is called response because it is exactly the ajax rest response. In java this would be a constructor to create a Node from a JsonResponse.