[m-rev.] For review 1/2: Beefed up the pqueue implementation.

[Julien Fischer]

On Wed, Nov 27, 2013 at 3:56 PM, Michael Richter <ttmrichter at gmail.com>wrote:

> Oops.  Forgot that this list is setting Reply-to to the person, not the
> list.  Sorry, Peter.
>
>
> On 27 November 2013 12:55, Michael Richter <ttmrichter at gmail.com> wrote:
>
>> On 26 November 2013 13:15, Peter Wang <novalazy at gmail.com> wrote:
>>
>>> On Sun, 24 Nov 2013 15:42:14 +1100, Paul Bone <paul at bone.id.au> wrote:
>>> > +
>>> > +:- pred pqueue.merge2(pqueue(K, V)::in, pqueue(K, V)::in, pqueue(K,
>>> V)::out)
>>> > +    is det.
>>> > +
>>> > +pqueue.merge2(empty,                   B,     B).
>>> > +pqueue.merge2(A at pqueue(_, _, _, _, _), empty, A).
>>> > +pqueue.merge2(pqueue(_, K, V, L, R),   !.PQ at pqueue(_, _, _, _, _),
>>> !:PQ) :-
>>> > +    pqueue.merge2(L, !PQ),
>>> > +    pqueue.merge2(R, !PQ),
>>> > +    pqueue.insert(K, V, !PQ).
>>>
>>> pqueue.merge2(A, B, C) :-
>>>     (
>>>         A = empty,
>>>         C = B
>>>     ;
>>>         A = pqueue(_, _, _, _, _),
>>>         B = empty,
>>>         C = A
>>>     ;
>>>         A = pqueue(_, K, V, L, R),
>>>         B = pqueue(_, _, _, _, _),
>>>         merge2(L, B, C0),
>>>         merge2(R, C0, C1),
>>>         insert(K, V, C1, C)
>>>     ).
>>>
>>
>> Personally I find the multiple functors easier to see that the switch has
>> been appropriately covered.
>>
>
It's det code the compiler will tell if if things haven't been
appropriately covered.

The coverage data is basically tabular in nature.  The linear format of
>> your choice here is IMO a poor fit.
>>
>
I prefer Peter's version -- it avoids several horrible looking head
unifications.  If nothing
else, they have a tendency to look nasty inside the debugger.  (Consider
what someone
browsing the a call to merge2 in inside mdb would have to deal with "B" or
whatever
the compiler expands "!.PQ at pqueue(_, _, _, _, _)" into.)

Cheers,
Julien.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20131127/517e3b42/attachment.html>