[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Jim Blandy <jimb_at_zwingli.cygnus.com>
Date: 2001-03-05 18:25:49 CET

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

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.