Re: Global replace: apr_hash_get/set(APR_HASH_KEY_STRING) -> svn_hash_get/set()

On Mon, Feb 11, 2013 at 5:32 PM, Julian Foad <julianfoad_at_btopenworld.com>wrote:

> I just noticed that in r1443387, Bert added:
>
>
> /* Shortcut for apr_hash_get() with a const char * key.
> *
> * @since New in 1.8.
> */
> #define svn_hash_gets(ht, key) \
> apr_hash_get(ht, key, APR_HASH_KEY_STRING)
> [...]
> #define svn_hash_sets(ht, key, val) \
> apr_hash_set(ht, key, APR_HASH_KEY_STRING, val)
>
> and it's now used in a few places.
>
> I like it, as brevity of low-level code is important for readability.
> I've had that thought many times but never done it.
>
> In my opinion we should simply replace all
> apr_hash_get/set(...APR_HASH_KEY_STRING...) with these, before branching
> 1.8.0, and be done with it, and not have a continuous trickle of changes.
>

+1

> We could bike-shed on the names, as always. 'gets' and 'sets' could imply
> a
> string *value* rather than the key (by analogy with stdlib gets() etc.),
> but they're good enough already -- we'd easily get used to them.
>

As soon as you want a more specific name, you will lose
the brevity. Maybe rename the parameters to key_string
and val_ptr plus extending the docstring? IDE users will
then be pretty safe due to all the Intellisense etc. magic.

> If no objections and nobody else does it, I can do it.
>

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*
http://www.wandisco.com/subversion/download
*