On Mon, Feb 11, 2013 at 5:32 PM, Julian Foad <[email protected]>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 *
