[OPEN-ILS-DEV] osrfHash: alternative implementation

[Mike Rylander]

On Mon, Mar 24, 2008 at 8:53 AM, Bill Erickson <erickson at esilibrary.com> wrote:
> Scott McKellar wrote:
>  > The attachments are an alternative implementation of the osrfHash
>  > code.  I believe that this implementation is simpler and more
>  > efficient than the existing one, while providing the same functionality
>  > (barring any bugs that I haven't found yet).
>  >
>  > These are NOT ready to be installed as replacements for the current
>  > code.  First of all, much of it is untested at this point.  Second,
>  > the rest of the code base isn't ready for it.  We would need to change
>  > any code that directly accesses the internal members of an osrfHash
>  > or osrfHashIterator.  In particular, some files reference the "current"
>  > member of an osrfHash Iterator.
>  >
>
>  I like the approach and I'm still digesting this to some degree.
>
>  (from your previous email)
>
>  "The osrfStringArray supports traversal of the keys in the same sequence in which they were added.  This ability is particularly important in the context of a jsonObject, where the sequence of entries in a series of name/value pairs may be significant."
>

I can't get too worked up over hash-key ordering ... associative
arrays don't have a guaranteed  order in other implementation that I
know of.

>  This isn't exactly the purpose.  The osrfStringArray was kept around mostly for speed.  When I was testing the hashes, there were lots of cases of "give me the list of keys for this hash".  Generally, it was (or seemed, with my contrived tests) faster to just keep the keys around than to create them on the fly.  However, the linked list implementation (as opposed to scanning through sparse arrays) will speed up the key generation a good bit, I would imagine.
>
>  (also)
>
>  "I suspect that the idea behind this is that we want the iterator to do
>  something sensible even if the item has been deleted (though it's not
>  clear to me whether this situation occurs in practice)."
>
>  And it shouldn't.  The intention was never to support mid-iteration item
>  deletions.  I think this is more of a unintentional (and hopefully
>  unused) side effect.
>
>
>  My feeling is that the ability to change a hash during iteration is a
>  bad thing and retaining sort order on the hash keys is not necessary.
>  If that simplifies your test implementation, great.  Regardless, I agree
>  we should start moving in this direction.  I can start by adding an
>  osrfHashIteratorKey() function to the current implementation so other
>  code can start using it.

I've been mulling this over for a couple days, and I've got to
disagree with Bill.  IFF mid-iteration changes (removal, in
particular, of a key) can be handled transparently and safely, and
that guarantee can be carved into the requirements for the API, then I
think it is a decidedly /good/ thing.

Imagine a method which provides a slimmed down interface to another
method.  It provides a subset of the functionality in return for
greater security.  Both accept a hash of named parameters.  Now, an
obvious implementation option would be to specify the acceptable
parameters available through the secure method and remove all keys
that do not match the list of provided parameter names by iterating
over the parameter hash and removing unwanted key/val pairs as we go.

When open-ils.permacrud is ported to C (it will be used heavily) this
option would be my first choice of algorithm -- it's straightforward
and easily understood.

A less specific argument goes something like "we should remove
undefined behavior if the path to such removal is clear and well
defined."

Thoughts?

-- 
Mike Rylander
 | VP, Research and Design
 | Equinox Software, Inc. / The Evergreen Experts
 | phone: 1-877-OPEN-ILS (673-6457)
 | email: miker at esilibrary.com
 | web: http://www.esilibrary.com