Re: CVS update: subversion/subversion/libsvn_fs dag.c dag.h

Greg Stein <gstein@lyra.org> writes:
> I'm not sure if Mike intended it or not, but we still have the need to open
> a node by ID rather than by revision/path. With this function, tree.c can
> accomplish that task.

This is still an unresolved issue. I'll send a separate message about
that.

But even if you were doing that, this docstring is poor:

    /* Allocate (from TRAIL->pool) and return a new dag_node_t structure
       *NODE, initialized with the values ID and FS, and setting
       *NODE->pool to TRAIL->pool. */

It's poor because it describes the function's implementation, instead
of its intended effect. A well-described interface is one that you
could use simply by reading the .h file. You shouldn't need to know
anything about dag_node_t's implementation. If you're trying to serve
Greg's needs, the docstring should say something like this:

   /* Set *NODE_P to a new dag node referring to the node in FS whose
      node revision ID is ID, as part of TRAIL. Allocate the new node
      in TRAIL->pool. */

Remember that the users of the interface are trying to write code that
won't need to be fixed when `internal' changes are made to dag.c. The
original docstring is written in a way that suggests that, if the
internals of dag_node_t were changed, that function would change to
match them. This is annoying for the interface's users. The second
docstring anchors the effect of the function in concepts that are
actually meaningful to the interface's users.
Received on Sat Oct 21 14:36:25 2006