API consistency? db.createNode vs. node.createRelationShipTo

[kontrafiktion]

As far as I understand the API (documentation):

GraphDatabase#createNode

only creates a node but does not persist it. On the other hand

Node#createRelationShipTo (/From)

not only creates the relationship but persists it as well. I stumbled upon this by accident, I "forgot" to save the relationship after creating it, but it was stored in the DB anyway.

I would propose to either very explicitly document that both "create..." methods behave differently or rename "createRelationship..." to to "saveRelationship..."

[aseemk]

Great q! I've had it on our roadmap for a while to have createNode() also persist -- I didn't think there was any reason to not persist if you're creating a node:

https://github.com/thingdom/node-neo4j/wiki/Roadmap

But just recently, in our own app, we might have a reason. So let me chew on this. Thanks for the feedback!

[blevine]

The behavior of createNode() can't be changed. You'll break binary compatibility for existing applications. For existing apps, this would probably result in the Node being persisted twice. Once during createNode() and then again during Node.save(). So I guess technically, this might not break anything, it would result in two REST calls where there was only one before. You already allude to the fact that this would be a breaking change in your comments in GraphDatabase._coffee. So even though in retrospect this might have been the correct behavior, it's not so important that it warrants breaking existing applications. IMHO, owners of public APIs should do everything in their power to not introduce breaking changes.

[kontrafiktion]

Considering that this is version 0.2.x, I wouldn't say that the API must not be broken.

If -- as assemk said above (#67 (comment)) -- there is a reason for having two API calls, one for creating and then one for persisting a node, the API for the node should perhaps stay as it is. But the API for the relationship could perhaps still be changed, by first adding the "saveRelationship..."-methods and deprecating the "create..."-methods. And in release 0.3 the create methods could be removed. Replacing these methods could probably be done in 5 minutes in any existing code base.

If there'd be a need to split up creating a relationship and persisting a relationship ... that API change would probably take much more time.

[blevine]

Are you saying that the API can be broken because the version number is not yet 1.0? I don't think this is a good criterion as I use very few 3rd party Node.js libraries that are 1.0 or greater. Or are you saying that it would be OK to break compatibility between minor versions (e.g. between 0.2.x and 0.3.x)? Either way, you really need to think long and hard before making any incompatible changes to a library that you have published and know is in use by others.

[aseemk]

Sorry I haven't had a chance to think about this issue more deeply, but I wanted to quickly jump in here for the compatibility/breaking-change question: I definitely value backwards-compatibility, and there's definitely value in breaking that sometimes for good cause.

I've been long thinking of some architectural changes that'd be good to make to this library, esp. as Neo4j 2.0 comes out with architectural changes there too (e.g. around indexing):

https://github.com/thingdom/node-neo4j/wiki/Roadmap

My thought has always been to save breaking changes and batch them together in major updates. This is similar to the approaches of many popular libraries, like jQuery, Express, etc.

We would indeed bump the major version in that case, e.g. to 1.0. It'd be nice if the library were already 1.x so that this could be 2.x, but alas, we can't change the past. =)

Edit: actually, no reason we can't skip 1.x and go directly to 2.x anyway, to reflect that this is major revision 2.

[aseemk]

On this note, I've created a new discussion group for discussing these kinds of things! Please consider joining. =)

https://groups.google.com/group/node-neo4j

[blevine]

Yes, I suppose it's fine if you need to make breaking changes when you bump to a new major release. But even those should be carefully considered. I think we're in agreement here.

[aseemk]

Good news — this inconsistency is getting tackled in the currently ongoing v2 redesign!

Sort of... methods to operate on nodes and relationships are going away, in favor of Cypher. So this inconsistency is going away then too. =P

Issue: #143 / PR: #145
WIP docs: https://github.com/thingdom/node-neo4j/blob/v2/API_v2.md

So going ahead and closing this as "won't fix", but feedback on the v2 redesign welcome!