Opened 8 years ago

Closed 6 years ago

#1290 closed defect (fixed)

Unit srfi-69: does not specify action when two merged hash tables contain the same keys

Reported by: Norman Gray Owned by:
Priority: minor Milestone: 4.13.0
Component: extensions Version: 4.10.x
Keywords: srfi-69 Cc:
Estimated difficulty: trivial

Description

The documentation for hash-table-merge says

(hash-table-merge HASH-TABLE-1 HASH-TABLE-2) procedure
Returns a new HASH-TABLE with the union of HASH-TABLE-1 and HASH-TABLE-2.

However this does not specify which hash table 'wins' when the two hash tables contain the same key. This procedure doesn't appear in SRFI 69, so that doesn't resolve it.

From experiment, and by inspection of the code, it is clear that hash-table-1 wins:

    % csi
    
    CHICKEN
    (c) 2008-2015, The CHICKEN Team
    (c) 2000-2007, Felix L. Winkelmann
    Version 4.10.0 (rev b259631)
    macosx-unix-clang-x86-64 [ 64bit manyargs dload ptables ]
    compiled 2015-08-04 on yves.more-magic.net (Linux)
    
    #;1> (use srfi-69)
    ; loading /Data/tools/chicken-4.10.0/lib/chicken/7/srfi-69.import.so ...
    ; loading library srfi-69 ...
    #;2> (define h1 (alist->hash-table '((a . 1) (x . 2))))
    #;3> (define h2 (alist->hash-table '((b . 1) (x . 3))))
    #;4> (define h3 (hash-table-merge h1 h2))
    #;5> (hash-table->alist h3)
    ((x . 2) (b . 1) (a . 1))
    #;6> 

But it would be good to document this, so that the user can know this is not an unspecified behaviour which may change in future. The same goes for

(alist->hash-table '((a . 1) (a . 2)))

Does this produce a hash table with a defined as 1 or 2 (it's 1, but the documentation doesn't guarantee this)


Incidentally (and at the risk of putting distinct notes into one ticket), it would be really attractive if this library defined a pure-functional hash-table-set which mapped (hash-table? any any) -> hash-table?

Change History (2)

comment:1 Changed 7 years ago by sjamaan

Estimated difficulty: trivial
Milestone: someday4.13.0

Thanks for reporting this, sorry that it went unnoticed for a while.

I think documenting this would be a good improvement.

Regarding the purely functional API: I don't think that would be a good idea for hash tables, as it implies copying the entire table which is rather expensive (avoiding this would imply a complete redesign). It would only make sense for smallish hash tables, but for those cases an alist will suffice just as well (and may be faster even), and we already have functional APIs for those.

comment:2 Changed 6 years ago by sjamaan

Resolution: fixed
Status: newclosed

Fixed with e311b61770a64fde0ba4503cea5930b4d74679c0 and r34689

Note: See TracTickets for help on using tickets.