Lucifers,
we have a pull request from Tim which adds a HashIterator for Clownfish hashes.
Unfortunately, we don’t get a notification to the dev list for Clownfish pull
requests yet.
https://github.com/apache/lucy-clownfish/pull/1
I had a quick review and the changes look solid. I especially like the thorough
tests. I’d only change two things:
1. If HashIterator was implemented directly in Hash.c, it would have access to
the static global for the hash tombstone and the HashEntry struct. This would
simplify the code.
2. Get_Key and Get_Value check whether the current key is a tombstone. This can
happen if the hash was modified during iteration. I’d probably have missed this
corner case, but if this happens, I’d throw instead of returning NULL.
Another thing, which is more of a discussion item, is whether Get_Key and
Get_Value should throw or return NULL if Next hasn’t been called yet or the
iterator is finished. Personally, I’d generally avoid null pointers and prefer
an exception. Then Get_Key would be guaranteed to return an object. Also, if
Get_Value returns NULL, it’s hard to tell whether this is a valid hash value or
not. (Get_Value should be made “nullable”, by the way.)
Nick
