The OrderedHash PMC provides keyed and indexed access to its elements. It's a
hash that remembers the order of its key/value pair additions and it's an
array that allows keyed access to (some of) its members.
While the naive approach might be to use both a hash and an array underneath,
this PMC uses a hash and mangles its keys to encode insertion order. I'm not
sure this is necessarily a good idea. I haven't studied the code in
sufficient detail to convince myself that the escaping system used (see the
Parrot_sprintf() call in set_pmc_keyed_int(), for example) is free of bugs;
there could be collisions.
The iteration code is complex, but that's due more to the design of our hashes
(src/hash.c), which expose way too much information. See the use of
key_next() in get_pmc_keyed(), for example.
The code has some cleverness; the way it handles negative indices is clean and
clear (once you know what it does). Again, see get_pmc_keyed_int():
n = h->entries;
if (idx < 0)
idx += n;
There's similar code in set_pmc_keyed_int(), but I don't understand quite what
the extra clause does, nor why we need to support it:
n = h->entries;
if (idx < -n)
idx = -idx - n - 1;
else if (idx < 0)
idx += n;
All of the vtable methods that access keys share their internal logic; there's
a lot of very-near dupication here. I've almost convinced myself that a
short macro would remove this duplication and help avoid bugs in maintenance.
The same type of duplication is evident in the exists() and defined() methods.
I decided to look into this code while tracking down its behavior for clone().
My questions are:
- should this method copy null values?
- are the semantics of delete() sane at all?
- why does this code access the contained hash directly instead of
through an
interface?
- how are the tests?
I welcome any and all comments on this file. I've already checked in some
minor cleanups, though they mostly affected formatting.
-- c
