snippetcsharpModerate
Optimize custom double.parse()
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.
would better be:
Additionally, I'm not sold on the name
Var
Use the
e.g.
should be
You should also use
e.g.
should be:
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.
should be:
Design
Why have you created the function
Why make
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.
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.