Address CR feedback: cleanup code, update README

README.md

@@ -37,69 +37,84 @@
 const dagPB = require('ipld-dag-pb')
 
 // then, to access each of the components
-dagPB.DAGNode.create // function to create DAGNodes (recommended usage)
+dagPB.DAGNode.create // function to create DAGNodes
 dagPB.resolver
 dagPB.util
 ```
 
 ## API
 
-### DAGNode Class
+### DAGNode
 
-Create a new DAGNode
+DAGNodes are created and manipulated with `DAGNode` class methods. You **can't instantiate a DAGNode directly** with `new DAGNode(...)`.
 
-```JavaScript
-dagPB.DAGNode.create([<data>, <[links]>, hashAlgorithm, callback]) 
-```
+You can incude it in your project with:
 
-#### TODO: update the functions docs from here on out
+```javascript
+const dagPB = require('ipld-dag-pb').DAGNode
+```
 
-#### `addNodeLink`
+#### create(data, dagLinks, hashAlg, callback)
 
-> creates a link on node A to node B by using node B to get its multihash
+Create a DAGNode.
 
-#### `addRawLink`
+```JavaScript
+DAGNode.create("data", (err, dagNode) => ...) 
+```
 
-> creates a link on node A to node B by using directly node B multihash
+#### addLink(dagNode, nameOrLink, nodeOrMultihash, callback)
 
-#### `updateNodeLink`
+Creates a link on node A to node B by using node B to get its multihash. Returns a *new* instance of DAGNode without modifying the old one.
 
-> updates a link on the node. *caution* this method returns a copy of the MerkleDAG node
+```JavaScript
+DAGNode.addLink(node, "Link1" (err, dagNode) => ...) 
+```
 
-#### `removeNodeLink`
+#### removeLink(dagNode, nameOrMultihash, callback)
 
-> removes a link from the node by name
+Removes a link from the node by name. Returns a *new* instance of DAGNode without modifying the old one.
 
-#### `removeNodeLinkByHash`
+```JavaScript
+DAGNode.removeLink(node, "Link1" (err, dagNode) => ...) 
+```
 
-> removes a link from the node by the hash of the linked node
+#### clone(dagNode, callback)
 
+Creates a clone of the MerkleDAG Node
 
-#### `clone`
+```JavaScript
+DAGNode.clone(node, (err, dagNode) => ...) 
+```
 
-> creates a clone of the MerkleDAG Node
+### DAGNode
 
-#### `size`
+The DAGNode instance returned by `DAGNode` class methods has the following properties. You **can't instantiate a DAGNode directly** with `new DAGNode(...)`, see [DAGNode.create()](#create) for details.
 
-> (property) size of the node, in bytes
+#### size
 
-#### `links`
+Size of the node, in bytes
 
-> (property) an array of `DAGLink`s belonging to the node
+```JavaScript
+const size = dagNode.size
+```
 
-#### `multihash(callback)`
+#### links
 
-> returns the multihash (default: sha2-256)
+An array of `DAGLink`s belonging to the node
 
-#### `getPBNode`
+```JavaScript
+const links = dagNode.links
+```
 
-> used internally
+#### multihash
 
-#### `makeLink`
+Returns the multihash (default: sha2-256)
 
-> used internally
+```JavaScript
+const multihash = dagNode.multihash
+```
 
-### DAGLink Class
+### DAGLink
 
 Create a new DAGLink
 

src/dag-node.js

@@ -14,8 +14,8 @@ class DAGNode {
 
     this._data = data || new Buffer(0)
     this._links = links || []
-    this._serialized = serialized || new Buffer(0) // TODO: default serialized object
-    this._multihash = multihash || new Buffer(0) // TODO: default multihash object
+    this._serialized = serialized
+    this._multihash = multihash
     this._size = this.links.reduce((sum, l) => sum + l.size, this.serialized.length)
     this._json = {
       data: this.data,

src/index.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const DAGNode = require('./utils.js')
+const DAGNode = require('./util.js')
 exports.DAGNode = DAGNode
 exports.DAGLink = require('./dag-link.js')
 exports.resolver = require('./resolver.js')

src/resolver.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const util = require('./utils').util
+const util = require('./util').util
 const bs58 = require('bs58')
 
 exports = module.exports

src/utils.js → src/util.js

@@ -8,8 +8,6 @@ const proto = protobuf(require('./dag.proto'))
 const DAGNode = require('./dag-node')
 const DAGLink = require('./dag-link')
 
-const hash = (type, data, cb) => multihashing(data, type, cb)
-
 const linkSort = (a, b) => {
   return (new Buffer(a.name || '', 'ascii').compare(new Buffer(b.name || '', 'ascii')))
 }
@@ -20,17 +18,14 @@ function cid (node, callback) {
 
 function create (data, dagLinks, hashAlg, callback) {
   if (typeof data === 'function') {
-    // empty obj
     callback = data
     data = undefined
   }
   if (typeof dagLinks === 'function') {
-    // empty obj
     callback = dagLinks
     dagLinks = []
   }
   if (typeof hashAlg === 'function') {
-    // empty obj
     callback = hashAlg
     hashAlg = undefined
   }
@@ -44,7 +39,6 @@ function create (data, dagLinks, hashAlg, callback) {
       return l
     }
 
-    // haadcode: are the .name vs .Name for backwards compatibility?
     const link = new DAGLink(l.name || l.Name,
                              l.size || l.Size,
                              l.hash || l.Hash || l.multihash)
@@ -59,11 +53,11 @@ function create (data, dagLinks, hashAlg, callback) {
     links: links
   }, (err, serialized) => {
     if (err) {
-      callback(err)
+      return callback(err)
     }
     multihashing(serialized, hashAlg, (err, multihash) => {
       if (err) {
-        callback(err)
+        return callback(err)
       }
       const dagNode = new DAGNode(data, links, serialized, multihash)
       callback(null, dagNode)
@@ -93,7 +87,6 @@ function addLink (dagNode, nameOrLink, nodeOrMultihash, callback) {
 
   if (newLink) {
     links.push(newLink)
-    sort(links, linkSort)
   } else {
     return callback(new Error('Link given as the argument is invalid'), null)
   }
@@ -163,7 +156,7 @@ function toProtoBuf (node) {
   if (node.data && node.data.length > 0) {
     pbn.Data = node.data
   } else {
-    pbn.Data = null//new Buffer(0)
+    pbn.Data = new Buffer(0)
   }
 
   if (node.links.length > 0) {

test/dag-node-test.js

@@ -90,7 +90,7 @@ module.exports = (repo) => {
         expect(err).to.not.exist.mark()
         expect(node.data.length).to.be.equal(0).mark()
         expect(Buffer.isBuffer(node.data)).to.be.true.mark()
-        expect(node.size).to.be.equal(0).mark()
+        expect(node.size).to.be.equal(2).mark()
 
         DAGNode.util.serialize(node, (err, serialized) => {
           expect(err).to.not.exist.mark()