[m-rev.] for review: Define equality for version_hash_tables

Julien Fischer jfischer at opturion.com
Mon May 27 13:44:24 AEST 2013


On Mon, 27 May 2013, Peter Wang wrote:

>>  %-----------------------------------------------------------------------------%
>> @@ -297,7 +302,9 @@ unsafe_new_default(HashPred) = unsafe_init(HashPred, 7, 0.9).
>>
>>  %-----------------------------------------------------------------------------%
>>
>> -num_buckets(HT) = size(HT ^ buckets).
>> +num_buckets(HT) = size(inner(HT) ^ ht_buckets).
>> +
>> +num_occupants(HT) = inner(HT) ^ ht_num_occupants.
>>
>
> I think the changes would be cleaner by (un)wrapping within the heads
> only, e.g.
>
>    num_occupants(wrap(HT)) = HT ^ ht_num_occupants.
>
>    copy(wrap(HT0)) = wrap(HT) :- ..

I would prefer:

     num_occupants(HT) = NumOccupants :-
         HTRep = inner(HT),
         NumOccupants = ^ ht_num_occupants.

or something along those lines.  While more verbose, it means that
introduced variable names will not be used in the debugger.

> where
>
>    :- func wrap(ht_inner(K, V)) = version_hash_table(K, V).
>    :- mode wrap(out) = in is det.
>    :- mode wrap(in) = out is det.
>
> Also, I would like to see any differences with hash_table.m minimised.

Seconded.

>> +:- pred version_hash_table_equals(version_hash_table(K, V)::in,
>> +    version_hash_table(K, V)::in) is semidet.
>
> You could name it "version_hash_table.equals".

Agreed.  Furthermore, the equality predicate needs to be exported from
the module since things will currently break if it isn't.

Cheers,
Julien.



More information about the reviews mailing list