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

On 3/20/07, Erik Huelsmann <ehuels@gmail.com> wrote:
> 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.
>
> With the help of Vlad I could prevent us having this code duplication
> without having the helper in our public library:
>
> I made all of it internal in the private/ part of our include area.
>
> How about that?
>
I have no objections against putting this helper to private/ part of
our include area. Feel free to commit :)

PS: I didn't reviewed patch carefully.

-- 
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org