Re: [PATCH] Followup to r29374.

On Fri, Feb 22, 2008 at 11:25 AM, Karl Fogel <kfogel_at_red-bean.com> wrote:
> Kamesh Jayachandran <kamesh_at_collab.net> writes:
> > I am not sure about the value of change!. I think this is needed only
> > at the API level need not be at the usage level inside functions.
> >
> > Even if we change all the occurance of 'apr_hash_t *' with
> > svn_mergeinfo_t we peek at the corresponding variable as pure hashes
> > so we don't gain anything by 'abstracting the types' alone without
> > abstracting its accessors.
> >
> > If we love abstracting this data-type we need to provide its own
> > accessor routines i.e we should not blatantly use apr_hash_get/set.
>
> This is something we're going to need to decide -- currently, we have
> been changing internal code to use the types too...

I'd be OK with leaving them as typedefs (just for documentation
purposes), or as adding accessors. The nice advantage of adding
accessors is that we could make them typesafe too (instead of
returning void *s that need to be cast...). It's just more work.
(And gets annoying if you want to use some random hash APIs.)

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org