6
\$\begingroup\$

I'm using the BigRational beta off the BCL CodePlex (bcl.codeplex.com), and I realized it had no parsing method, so I tried to write one. However, it's quite inefficient (5.5ms for a 254 character number). That's over 45x slower than the BigInteger implementation. I'd like to lower it to .5ms if that's even possible.

private static Regex DigitDotDigit = new Regex(@"^(\-|\+)?(\d+\.\d+)$", RegexOptions.Compiled);
private static Regex PlainDigit = new Regex(@"^(\-|\+)?\d+$", RegexOptions.Compiled);
private static Regex DigitSlashDigit = new Regex(@"^(\-|\+)?\d+/\d+$", RegexOptions.Compiled);
private static Regex DotDigit = new Regex(@"^(\-|\+)?(\.\d+)", RegexOptions.Compiled);
private static bool RegexInitiated = false;
public static bool TryParse(string parse, out BigRational result)
{
    if (DigitDotDigit.IsMatch(parse))
    {
        int zeros;
        bool isNegative = false;
        string[] parts = parse.TrimStart('+').Split('.');
        parts[1] = parts[1].TrimEnd('0');
        if (parts[0].StartsWith("-"))
        {
            isNegative = true;
            parts[0] = parts[0].Substring(1);
        }
        BigRational whole = new BigRational(BigInteger.Parse(parts[0]), BigInteger.Zero, BigInteger.One);
        BigRational decimalPart = new BigRational(BigInteger.Parse(parts[1]), BigInteger.Zero, BigInteger.One);
        zeros = parts[1].Length - parts[1].TrimStart('0').Length;
        toSubtract = toSubtract + zeros;
        if (zeros > 0)
        {
            toSubtract = toSubtract - 1;
        }
        while (toSubtract != 0)
        {
            decimalPart /= 10;
            toSubtract = toSubtract - 1;
        }
        result = whole + decimalPart;
        if (isNegative)
        {
            result = -result;
        }
        return true;
    }
    else if (DotDigit.IsMatch(parse))
    {
        return TryParse("0" + parse, out result);
    }
    else if (PlainDigit.IsMatch(parse))
    {
        parse = parse.TrimStart('+');
        if (parse.StartsWith("-"))
        {
            result = new BigRational(-BigInteger.Parse(parse), BigInteger.Zero, BigInteger.One);
            return true;
        }
        result = new BigRational(BigInteger.Parse(parse), BigInteger.Zero, BigInteger.One);
        return true;
    }
    else if (DigitSlashDigit.IsMatch(parse))
    {
        string[] parts = parse.TrimStart('+').Split('/');
        if (parts[0].StartsWith("-"))
        {
            parts[0] = parts[0].Substring(1);
            result = -(new BigRational(BigInteger.Parse(parts[0]), BigInteger.Parse(parts[1])));
            return true;
        }
        result = new BigRational(BigInteger.Parse(parts[0]), BigInteger.Parse(parts[1]));
        return true;
    }
    result = BigInteger.Zero;
    return false;
}
\$\endgroup\$
8
  • \$\begingroup\$ I think that before worrying about performance, you should make sure that your code actually works correctly (hint: try parsing numbers like 0.1, or even just 1). And you should also include the whole method, so that we don't have to guess what the rest of it contains (even if it's quite obvious). \$\endgroup\$
    – svick
    Commented Mar 17, 2013 at 17:25
  • \$\begingroup\$ Whoops, missed the copy and paste. Wait one second. And it worked fine in ~25 tests that I did... \$\endgroup\$ Commented Mar 17, 2013 at 17:52
  • \$\begingroup\$ @svick Fixed that behaviour. Should've remembered to use >=. \$\endgroup\$ Commented Mar 17, 2013 at 17:57
  • \$\begingroup\$ It's still not completely fixed. Try 0.1001. \$\endgroup\$
    – svick
    Commented Mar 18, 2013 at 1:13
  • \$\begingroup\$ Setting a value in the middle of an expression is confusing, you shouldn't do that. \$\endgroup\$
    – svick
    Commented Mar 18, 2013 at 20:59

1 Answer 1

2
\$\begingroup\$

The answer, I found out now, was that if I moved the Regexes to private fields, and then used those, after the first use it will radically drop down to .4ms, and that's including a WriteLine call. So the real hotspot in this method was the Regex. Thanks everyone!

\$\endgroup\$
2
  • 1
    \$\begingroup\$ You might also want to look into compiling your regex. Look for the section "Interpreted vs Compiled Regular Expressions". Compiling them is a huge speed improvement. \$\endgroup\$ Commented Mar 18, 2013 at 13:55
  • \$\begingroup\$ Thought it did that by default, thanks for warning me :) \$\endgroup\$ Commented Mar 18, 2013 at 16:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.