Ticket #672 (closed defect: wontfix)

Opened 1 year ago

Last modified 1 year ago

tango.convert.Integer.toLong throws errors inconsistently regarding whitespace

Reported by:	Cyborg16	Assigned to:	kris
Priority:	minor	Milestone:	0.99.3
Component:	Core Functionality	Version:	0.99.2 Don
Keywords:	tango.convert.Integer toInt toLong throw whitespace	Cc:

Description

tango.convert.Integer.toLong throws an error when it doesn't consume all of its input string. Since trim() checks prefixes and removes whitespace extra whitespace at the beginning of the string doesn't cause toLong to throw an error, but extra whitespace at the end of the input string does.

The main problem with this is inconsistency. The idea of throwing an error when not all of the input is consumed is good to spot errors which would likely otherwise be overlooked. Maybe instead of just throwing an error if not all the input has been consumed it should check whether or not the remaining text is only whitespace?

Here's a version which does that (based on 0.99.2 Don):

long toLong(T) (T[] digits, uint radix=0)
{
        uint len;

        auto x = parse (digits, radix, &len);
        while (len < digits.length) {
            if (digits[len] == ' ' || digits[len] == '\t') ++len;
            else throw new IllegalArgumentException ("Integer.toLong :: invalid literal");
        }
        return x;
}

Change History

(follow-up: ↓ 2 ) 10/09/07 12:03:24 changed by kris

owner changed from sean to kris.

toLong() is a wrapper around parse(). If you need deeper information about what was consumed, then parse() would be a good place to extract that from.

We're a bit wary of introducing potential special-cases like this, since they may cause other kinds of problems, and produce inconsistencies elsewhere. For example, toLong would now do something quite different to parse() instead of executing a subset. A bit like that 'gopher' game, if you know what I mean? Another alternative is to pass some kind of flag around to indicate trailing whitespace should be consumed, but then you wind up with weak parameters that are only used in certain special cases.

The best solution is typically to trim the subject text first, and give toLong() the resultant slice: Util.trim() can do this for you. Hope that helps?

(in reply to: ↑ 1 ) 10/11/07 06:29:43 changed by Cyborg16

Well, I decided the functions didn't quite do what I wanted so I wrote my own. Feel free to use or not, and I don't mind doing a bit of work on the functions first if you give me some guidelines.

/** Templated read-int function to read various types of integer.
 *
 * Actually a reimplementation of tango.text.convert.Integer toLong and parse functions.
 */
TInt toTInt(TInt) (char[] str) {
    bool sign;
    uint radix, ate, ate2;
    
    ate = trim (str, sign, radix);
    ulong val = convert (str[ate..$], radix, &ate2);
    ate += ate2;
    
    while (ate < str.length) {
        if (str[ate] == ' ' || str[ate] == '\t') ++ate;
        else throw new Exception ("mde.mergetag.readStringTps.toTInt: invalid integer");
    }
    
    if (val > TInt.max) throw new Exception ("mde.mergetag.readStringTps.toTInt: integer out of range");
    if (sign) {
        long sval = cast(long) -val;
        if (sval > TInt.min) return cast(TInt) sval;
        else throw new Exception ("mde.mergetag.readStringTps.toTInt: integer out of range");
    }
    return cast(TInt) val;
}

Ticket Navigation

Ticket #672 (closed defect: wontfix)

tango.convert.Integer.toLong throws errors inconsistently regarding whitespace

Description

Change History

(follow-up: ↓ 2 ) 10/09/07 12:03:24 changed by kris

(in reply to: ↑ 1 ) 10/11/07 06:29:43 changed by Cyborg16

10/17/07 13:24:46 changed by Cyborg16

11/02/07 23:21:11 changed by kris

11/03/07 18:50:02 changed by larsivi

Download in other formats: