Code review comment for lp:~zorba-coders/zorba/identifiers

> <p>Please see the <a href="../../html/data_lifecycle.html">data lifecycle
> documentation</a> about details on management and manipulation of
> collections.</p>
> =>
> <p>Please see the <a href="../../html/data_lifecycle.html">data lifecycle
> documentation</a> about details on storing nodes in collections or as
> documents.</p>

Done.
>
> Computes an immutable and opaque node reference
> =>
> Returns an immutable and opaque node reference...
>
Done.

> The function returns the empty sequence if the node
> that is referenced was deleted.
> =>
> The function returns the empty sequence if the node
> that is referenced does not exist (e.g. because it was deleted or the
> temporary node is not available anymore)
>
Done.

> zerr::ZAPI0028
> =>
> zerr:ZAPI0028
>

Done.

> Can any of the functions return ZAPI0029? If so, it's not documented. Same for
> ZAPI0030.
>

They cannot be raised by the functions in this module. However they can be raised when some of the simple_store methods are called directly, like i do in my pul module.
An example could be a PUL which requires to insert a node with an identity which is already assigned to another node. (Either the same PUL/operation is applied twice or the user is using crafting ids). In those function these errors are either catched and wrapped into other errors or thrown.

>
> fix indentation in NodeByReferenceIterator (two spaces)
>
Done
> Also, add a space before and after the assignment
Done, also in the other module
>
> Fix comment in FnGenerateIdIterator => remove e.g. z0.1.1.c.50
Done.
>
> Why is hasReference still needed?
I use this in my pul module to verify whether a node has a reference or not.

>
> What is the following change in node_items.cpp for?
> 513 - theTypesMap = new
> NodeTypeMap(source->theTypesMap->object_count(), false);
> 514 + theTypesMap = new NodeTypeMap(source->theTypesMap->size(),
> false);

I think Markos changed it when he replaced one of my tree-based maps with a more efficient hash-based map.

> What is the CopyMode's theCopyReference member for? Isn't it always false?
Yes in the current module it is. Should I move it in the branch which uses it/by itself?