HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpModerate

Optimize custom double.parse()

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
customdoubleoptimizeparse

Problem

In my company's code, they use double.tryParse() which is quite good but sets too much security for our needs. As we sometimes have to parse a few billion strings, I came up with this code which is a little bit faster (10%) but I have the feeling that it's not at its best... Unfortunately, this kind of stuff require more knowledge than I have.

private static double GetDoubleValue(string input)
{
    double n = 0;
    int decimalPosition = input.Length;
    bool separatorFound = false;
    bool negative = input[0] == '-';
    for (int k = (negative ? 1 : 0); k  0)
    {
        if (exp%2 == 1)
            result *= num;
        exp >>= 1;
        num *= num;
    }
    return result;
}

Solution

Naming

Avoid single-letter or shortened form variable names. Extra characters on a variable's name are free, and work wonders for a maintenance programmer down the line.

e.g.

double n = 0;


would better be:

double output = 0;


Additionally, I'm not sold on the name GetDoubleValue. At no point does this method retrieve the value from anywhere. What this method does is conversion, so it should be appropriately named as such.

Var

Use the var keyword when defining local variables where the right hand side of the definition makes the type obvious. This looks cleaner and saves time when it comes to changing types during refactoring.

e.g.

bool separatorFound = false;


should be

var separatorFound = false;


You should also use var when declaring foreach and for loop iterators.

e.g.

for (int k = (negative ? 1 : 0); k < input.Length; k++)


should be:

for (var k = (negative ? 1 : 0); k < input.Length; k++)


Braces

Always use braces for if statement bodies. It's cleaner, more explicit in intent, and it's quicker should you later decide to add extra lines to the body.

e.g.

if (!char.IsDigit(c))
    return Double.NaN;


should be:

if (!char.IsDigit(c))
{
    return Double.NaN;
}


Design

Why have you created the function CustomPow when all you're really doing is repeatedly multiplying by 10? It seems this is a case of over-abstraction, unless you plan to use the method elsewhere too.

Why make GetDoubleValue private? It sounds like a useful function you may want to use elsewhere or in other projects. I suspect it's private because it is in a class it does not belong in, and you don't want somebody calling StringLoader.GetDoubleValue or something like that.

Put it in a utility class or an extension method somewhere, it doesn't make sense as a method of your string loader class or whatever.

Code Snippets

double n = 0;
double output = 0;
bool separatorFound = false;
var separatorFound = false;
for (int k = (negative ? 1 : 0); k < input.Length; k++)

Context

StackExchange Code Review Q#75791, answer score: 14

Revisions (0)

No revisions yet.