[m-rev.] Proposed changes to pqueue.m

[Julien Fischer]

Hi,

On Mon, 18 Nov 2013, Paul Bone wrote:

>>  @@ -47,6 +51,15 @@
>>   :- pred pqueue.insert(K::in, V::in, pqueue(K, V)::in, pqueue(K, V)::out)
>>       is det.
>>
>>  +    % Extract the smallest item from the priority queue without removing
>> it.
>>  +    % Fails if the priority queue is empty.
>>  +    %
>>  +:- pred pqueue.peek(pqueue(K, V)::in, V::out) is semidet.
>>  +
>>  +    % As above, but calls error/1 if the priority queue is empty.
>>  +:- func pqueue.det_peek(pqueue(K, V)) = V.
>>  +:- pred pqueue.det_peek(pqueue(K, V)::in, V::out) is det.
>>  +
>>       % Remove the smallest item from the priority queue.
>>       % Fails if the priority queue is empty.
>>       %
>
> How long are lines in your text editor?  We like to limit lines to 76
> columns so taht a diff plus an e-mail reply doesn't cause wrapping.

Do we?  According to the Mercury project's coding standard we limit them
to 79 characters.  And indeed, that's what most of the code does.

> Why isn't the K returned in the predicate version of peek.  I'd imagine in
> some cases it'd be useful.  If a caller doesn't want it then they can pass
> the '_' variable.

I suggest having three versions:

  - peek, which returns both the key and the value
  - peek_key, which returns the smallest key
  - peek_value, which returns the value corresponding to the smallest key

...

> Comments on declarations should always include an extra line with a %
> character.
>
>>  +%---------------------------------------------------------------------------%
>>  +
>>  +pqueue.det_peek(PQ) = V :-
>>  +    pqueue.det_peek(PQ, V).
>>  +
>>  +pqueue.det_peek(PQ, V) :-
>>  +    ( pqueue.peek(PQ, T) ->
>>  +        V = T
>>  +    ;
>>  +        error("pqueue.det_peek/2: empty priority queue")
>>  +    ).
>>  +
>
> Since this module was written we've introduced 'implementation defined
> literals'.  They include $module and $pred and can be used to print the
> module and predicate name in errors.  Also see the unexpected predicate in
> the require module.  So you can write:
>
>    unexpected($module, $pred, "empty priority queue")

I prefer "unexpected($file, $pred, ..." since $pred already includes the
module name.

Cheers,
Julien.