On Wed, Mar 31, 2010 at 11:34:35AM +0200, Bert Huijben wrote:
>
>
> > -----Original Message-----
> > From: Greg Stein [mailto:gstein_at_gmail.com]
> > Sent: woensdag 31 maart 2010 2:47
> > To: dev_at_subversion.apache.org
> > Subject: redundant path functions
> >
> > The following functions seem very redundant. Because each is slightly
> > different, I always have to compare/contrast them to isolate their
> > differences. It would be much better if we could pick JUST ONE, and
> > run with that:
> >
> > svn_*_is_child()
> > svn_*_is_ancestor()
> > svn_*_skip_ancestor()
> >
> > I do realize that we published the svn_dirent_* functions in 1.6, but
> > we don't have to carry their analogues to the new functions. (and
> > possibly even deprecate the 1.6 funcs)
> >
> > It seems that the is_child variant is the only function needed. The
> > is_ancestor() is merely is_child() != NULL, and the skip_ancestor is
> > simply pool=NULL.
> >
> > Am I missing something, and/or can/should we remove this redundancy?
>
> There is one difference between is_child and the ancestors functions: How
> they handle the case where the path is the ancestor itself.
>
> The is_child function returns NULL, while is_ancestor returns true.
> Skip ancestor removes the ancestor (prefix) and returns "" for the ancestor
> itself and leaves paths that don't have the ancestor as-is.
I too remember being confused as to which interface I should be using.
And I think having is_ancestor not be the reverse of is_child is a bad idea.
We'll likely find mis-use of either interface in the code base, where
the author didn't realise the subtle semantics of the interface.
+1 on settling on one variant (I don't really care which), deprecating
the others, and updating callers.
Stefan
Received on 2010-03-31 13:44:18 CEST