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

Re: [PATCH] svn_wc_ventry(_ex) to eliminate heavily repeated code

From: Erik Huelsmann <ehuels_at_gmail.com>
Date: 2007-03-19 21:47:12 CET

On 3/19/07, Ivan Zhakov <chemodax@gmail.com> wrote:
> On 3/17/07, Erik Huelsmann <ehuels@gmail.com> wrote:
> > In libsvn_wc, we have a number of patterns which are extremely common,
> > yet verbosely repeated every time. One of them is addressed in the
> > patch attached.
> >
> > The pattern is:
> >
> > - get an entry using svn_wc_entry()
> > - check for a non-NULL entry and,
> > return a not-versioned error if the entry *is* NULL
> >
> > The only thing I could figure out why this would be is that we want to
> > have file and line numbers in the error message, so, I introduced a
> > macro and a new function.
> >
> > The function svn_wc_ventry_ex (ventry as for 'valid entry') which
> > takes a file name and a line number. The macro svn_wc_ventry()
> > forwards to the function, adding the filename and lineno.
> >
> > I know it isn't our usual pattern, but it's one of the ways I see to
> > make libsvn_wc a bit more readable.
> >
> > BTW: This patch has 2 failing tests, because of changed error output.
> > No real problems, as far as I can tell.
> >
> > Comments?
> >
> I like this idea, but I prefer to add this helper locally to libsvn_wc
> and libsvn_client. Because exporting and maintaining such helpers
> isn't good way. Interface of library should be minimal as possible. So
> I propose to create svn_wc__ventry_ex() and
> svn__clinet_wc_ventry_ex().
> Yes this introduce *some* code duplication, but I think it's better
> than exporting helpers from our library.

But we already do in the context of error handling: see svn_error__locate().

I also wouldn't like to export helpers, but I think it's 'madness' to
start maintaining code in several layers separately. What if we were
to change error creation to move away from global variables and use
the same scheme as the one here?

Next to that, I tink that documenting something as 'internal' should
be enough for people not to use it... It's not like they have no other
option.

You are aware that all svn_*__ symbols are exported from the libraries
(at least on Linux) too?

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 19 21:47:25 2007

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