[m-rev.] for review: new implementation of string__format in mercury.

[Fergus Henderson]

On 31-Jan-2002, p.incani <paulai at ugrad.unimelb.edu.au> wrote:
> Removed the old implementation of string__format because it relies on C's
> printf function - this causes problems when Mercury is compiled to Jave or

s/Jave/Java/

> Index: string.m
...
> @@ -1269,7 +1273,7 @@
>  	( ['*'] ->
>  		{ PolyTypes0 = [i(Prec0) | PolyTypes1] ->
>  				% XXX Best done in C
> -			Prec = to_char_list(int_to_string(Prec0)),
> +			Prec = Prec0,

Is the XXX comment still appropriate?
Perhaps that comment should be deleted now.

> +	%
> +	% Format a signed int (d,i).
> +	%
> +:- func format_int(list(char), maybe(int), maybe(int), int) = string.
> +format_int(Flags, Width, Prec, Int) = String :-
> +		%
> +		% Find integers absolute value, and take care of special

s/integers/the integer's/
s/special/the special/

> +	%
> +	% Format an unsigned int, unsigned octal, unsigned hexadecimal (u,o,x,X).

s/octal,/octal, or/

> +format_unsigned_int(Flags, Width, Prec, Base, Int, Prefix) = String :-
> +		%
> +		% Find integers absolute value, and take care of special

see above

> +:- func size_of_required_exponent(string, int) = int.
> +size_of_required_exponent(Float, Prec) = Exponent :-
> +	UnsafeExponent = decimal_pos(Float),
> +	UnsafeBase = calculate_base_unsafe(Float, Prec),
> +		%
> +		% Is mantissa one digit long?
> +		%
> +	MantAndFrac = string__words(is_decimal_point, UnsafeBase),
> +	list__index0_det(MantAndFrac, 0, MantissaStr),

Code similar to this occurs in several places.
It would be nicer to define a predicate

	:- pred split_at_decimal_point(string::in, string::out, string::out).
	split_at_decimal_point(MantAndFrac, Mantissa, Fraction) :- ...

and use that.

> +	( string__length(MantissaStr) > 1
> +	->

The comment says "is mantissa one digit long",
but the code checks for the mantissa being more than one digit long.
Either the code or the comment should be changed so that they
are consistent.

> -:- pred using_sprintf is semidet.

For the C back-end, I suspect the old implementation of string__format
using sprintf() is more efficient.

So it might be better to conditionally enable your version only
for the non-C back-ends, using this `using_sprintf' predicate.

> +:- func change_to_g_notation(string, int, string, list(char)) = string.
...
> +			FormattedFloat0 = change_precision(abs(DecimalPos)-1+Prec, Float)

Use whitespace around operators such as `-' and `+'.

Please post another diff (both a relative diff and a full diff)
when you have addressed the review comments posted so far.

-- 
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
--------------------------------------------------------------------------