FS tree walking (was: Re: wcprops)

On Mon, May 20, 2002 at 03:22:09PM -0500, cmpilato@collab.net wrote:
> Ben Collins-Sussman <sussman@collab.net> writes:
> > Greg Stein <gstein@lyra.org> writes:
>...
> > > The flat list seems fine. I dislike using dir_delta as a mechanism to do a
> > > walk, though. It smacks too much of "everything is a nail for dir_delta to
> > > smack."
>..
> > manually walking an fs tree? Is it *that* much faster than using
> > dir_delta against rev 0?
>
> Much faster? I doubt it. At least, from the server side, dir_deltas
> is doing the the same things: svn_fs_dir_entries; loop; recurse.

It isn't about speed; it is about complexity. Setting up a call to
dir_delta, and the corresponding editor, is a PITA. And its semantic
overhead is way larger than the problem being solved.

Consider somebody coming along and reading the code. Consider how much they
are going to have to wade through to realize that you're doing a simple walk
over revision REV of the tree. At first, they're going to see a comparison
of two trees, then they'll realize that one is rev 0, and after another
ka-chunk, they'll realize you're just walking a tree.

That's a lot of brain effort for a simple "walk" operation.

Consider the dir_delta function, and a hypothetical walk function:

svn_error_t *
svn_repos_dir_delta (svn_fs_root_t *src_root,
                     const char *src_parent_dir,
                     const char *src_entry,
                     svn_fs_root_t *tgt_root,
                     const char *tgt_path,
                     const svn_delta_edit_fns_t *editor,
                     void *edit_baton,
                     svn_boolean_t text_deltas,
                     svn_boolean_t recurse,
                     svn_boolean_t entry_props,
                     svn_boolean_t use_copy_history,
                     apr_pool_t *pool);

compare that to:

svn_error_t *
svn_repos_walk_tree (svn_fs_root_t *root,
                     const char *path, /* starting path locn */
                     svn_repos_walk_func_t callback,
                     void *cb_baton,
                     apr_pool_t *pool);

where you have:

typedef svn_error_t * (*svn_repos_walk_func_t) (const char *path,
                                                void *baton,
                                                apr_pool_t *pool);

As soon as a reader sees the call to walk_tree, they will know exactly what
is going on. No further reading necessary.

[ and if anybody says "well, just insert a comment before the call to
  dir_delta to explain that a walk is occurring", I'm going to have to break
  their legs. as I've said before, comments can be crutches to writing clear
  code in the first place. this is a perfect example of that. ]

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org