[m-rev.] diff: more code cleanups for integer.m

Fergus Henderson fjh at cs.mu.OZ.AU
Mon Dec 22 23:25:22 AEDT 2003


On 22-Dec-2003, Julien Fischer <juliensf at students.cs.mu.OZ.AU> wrote:
> 
> More code cleanups for the integer module.  This does not change
> any algorithms.
> 
> library/integer.m:
> 	Use distinct variable names. e.g X and Y rather than X1 and X2
> 
> 	Fix indentation so that it adheres more closely to what
> 	is specified in the coding standards.
> 
> 	Remove a redundant if-then-else.
> 
> 	While converting a list of digits to a string call error/1
> 	if something goes wrong rather than appending "Woops" to
> 	the string and continuing on.
> 
> 	Replace calls to int__to_float/2 with calls to float__float/1.
> 
> 	Fix an incorrect reference in an error message.

There were some additional changes not mentioned in this log message.
One was to convert quite a few if-then-elses from the "... -> ... ; ..." form
to the "if .. then ... else ..." form.  Another was to change

	foo(...) -->
		[].

to

	foo(...) --> [].

in several places.

That's OK this time... but in future, if you're going to make changes
like that, it's much better to first put some sort of explicit policy
about it in the Mercury coding guidelines.  Otherwise there is a danger
of continual churn, as each developer rewrites things in his/her favourite
syntax!

> +quot_rem(U, V, Quot, Rem) :-
> +	( U = i(_, [UI]), V = i(_, [VI]) ->
> +		Quot = shortint_to_integer(UI // VI),
> +		Rem  = shortint_to_integer(UI rem VI)
> +	;
> +		V0 = head(V),
> +		( V0 < basediv2 ->
> +			M = base div (V0 + 1),
> +				quot_rem_2(integer__zero,
> +					mul_by_digit(M, U),
> +					mul_by_digit(M, V), QuotZeros, R),
> +			Rem = div_by_digit(M, R)

The call to quot_rem_2 here is incorrectly intended.

>  :- func string_to_integer_acc(list(char), integer) = integer.
>  :- mode string_to_integer_acc(in, in) = out is semidet.
> 
>  string_to_integer_acc([], Acc) = Acc.
>  string_to_integer_acc([C | Cs], Acc) = Result :-
> -    (char__is_digit(C) ->
> -	char__to_int(C, D1),
> -	char__to_int('0', Z),
> -	Dig = pos_int_to_digits(D1 - Z),
> -	NewAcc = pos_plus(Dig, mul_by_digit(10, Acc)),
> -	Result = string_to_integer_acc(Cs, NewAcc)
> -    ;
> -	fail
> -    ).
> +	char__is_digit(C),
> +	Digit0 = char__to_int(C),
> +	Z = char__to_int('0'),
> +	Digit = pos_int_to_digits(Digit0 - Z),
> +	NewAcc = pos_plus(Digit, mul_by_digit(10, Acc)),
> +	Result = string_to_integer_acc(Cs, NewAcc).

The if-then-else here is not unnecessary.  It acts as sequential conjunction,
and it is needed to guarantee termination with --reorder-conj,
so it should not be removed.  Without it, the value of `Digit0 - Z'
might be negative, and the call to pos_int_to_digits might not terminate.

So please restore the if-then-else, and add a comment explaining why it is
needed.

Otherwise that looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list