fortran-lang / stdlib

Fortran Standard Library
https://stdlib.fortran-lang.org
MIT License
1.02k stars 161 forks source link

Remove other derived type from hashmap #843

Open chuckyvt opened 4 days ago

chuckyvt commented 4 days ago

This is a proposed update to the hashmap routines to remove the 'other' derived type that is currently required to store values. I reviewed the PR history related to the hashmap routines to try to understand the reason for using the other derived type. I don't see a clear reason other than perhaps it was thought that directly using unlimited polymorphic type would require use of select type in the code? I should note up front that this change is not backwards compatible with existing code, as the get_other_data function will now require as class(*), allocatable data type in the other field, which is discussed a bit more below. But I do believe it simpler and a better long-term approach.

For an example. Currently to set a value, and then retrieve it, the code looks like:

type(other_type)      :: other
class(*), allocatable  :: data
type(chaining_hashmap_type) :: map

call set(other, 4.0)
call map%map_entry(key, other)

call map%get_other_data(key, other)
call get(other, data)

select type (data)
    type is (real)
        print *, 'Value = ', data
end select

This will simply too

class(*), allocatable  :: data
type(chaining_hashmap_type) :: map

call map%map_entry(key, 4.0)

call map%get_other_data(key, data)

select type (data)
    type is (real)
        print *, 'Value = ', data
end select

As mentioned above, this change is not backwards compatible with existing code that uses the other type. The map_entry part will function correctly, however the get_other_data routine will require a class(*), allocatable type data. I believe the hashmaps are still listed as experimental, and thus subject to change, so would like to think this isn't too detrimental.

I am submitting this in draft form, as the specification still need updating.

Note, would like to give credit to @LKedward, as his fhash package was used as a reference in developing this PR.

jvdp1 commented 4 days ago

@chuckyvt The other_type was also questionned during the implementation of the hashmap modules: see here, here, here, here, here, here, here, here, here, here, here.

I probably missed some coments on this topic. But it seems that the main concern regarding other_type vs class(*) is the performance.