Build a double from a stream of chars

Question

Back in my calculator post, janos suggested I either find a better way to read a double from a StreamReader or else go the whole way and build a double from scratch. I looked through a few streams, but the only one I found that supported reading a double was the BinaryReader. This, unfortunately, reads exactly 8 bytes, and the data is stored as a char (or rather, as an int representation of a char), so I could enter the equation "5+5", and it would crash when it read the "+". If you know of a better solution, I would prefer using it.

private double GetNextNumber(StreamReader dataStream)
{
 double value = 0;
 int decimalLevel = 0;
 while (true)
 {
 if (!"0123456789.".Contains((char)dataStream.Peek()))
 {
 break;
 }
 char token = (char)dataStream.Read();
 if (token == '.')
 {
 if (decimalLevel == 0)
 {
 ++decimalLevel;
 continue;
 }
 throw new InvalidDataException("Invalid number format.");
 }
 int digit = int.Parse(token.ToString());
 value = value * 10 + digit;
 if (decimalLevel != 0)
 {
 ++decimalLevel;
 }
 }
 return value / Math.Pow(10, decimalLevel - 1); // use "- 1" because value increments just before ending loop
}

Please analyze my code as thoroughly as the compiler does.

• Snowbody: Even though this has a bug, it's not worth closing. You thought it was working code, and you asked for comments based on it.
• Snowbody: Still, you shouldn't edit the existing code. I'm reverting it as per meta.codereview.stackexchange.com/questions/1763/…

Answer 1

You should avoid while (true) { ... } loops wherever it is possible to do so. Here, it's very obviously avoidable!

while ("0123456789.".Contains((char)dataStream.Peek()))
{
 ...
}

This parser leaves much to be desired in terms of handling

• negative numbers (or you should document the fact that they are unsupported)
• e notation for powers of ten
• overflow and small numbers, due to excessive multiplying and dividing by powers of ten

Personally, I would avoid the entire headache by appending all plausible-looking characters to a StringBuilder, then calling double.Parse(string).

• Millie Smith: I had a guy once tell me on SO that while (true) { if (condition) { break; } ... } was a preference of style. I just didn't even respond. Gave up and left.

Answer 2

You're multiplying the integer part of the number by 10^{decimalLevel-1} and then dividing it again. That seems to be needlessly introducing round-off error. How about splitting the code into "before decimal point" and "after decimal point" and factoring out common parts into a new method? Also that would make it easier to handle scientific notation, which your code currently doesn't.

Also, I have a problem with

int digit = int.Parse(token.ToString());

It would be better to use an array or hashtable, rather than making a new temporary string in order to call int.Parse on it.

Answer 3

I think your original code that used double.TryParse is fine. In some languages (like C), there are methods that try to find a number at the start of the given string. This is annoying when the whole string you have should be a number, but it could be useful here.

Unfortunately for you, there is no such method in C#, hence my conclusion that your original code was okay.

Answer 4

This line has a bug when returning a value with no decimal value, as pointed out by mjolka:

return value / Math.Pow(10, decimalLevel - 1);

It evaluates to value / 10 ^ -1, which essentially returns value * 10.

This can be fixed in either of two ways:

return value / Math.Pow(10, decimalLevel == 0 ? decimalLevel : decimalLevel - 1);

Or, replace this section:

 value = value * 10 + digit;
 if (decimalLevel != 0)
 {
 ++decimalLevel;
 }
 }
 return value / Math.Pow(10, decimalLevel - 1); // use "- 1" because value increments just before ending loop
}

With this:

 if (decimalLevel == 0)
 {
 value = value * 10 + digit;
 }
 else
 {
 value += digit / Math.Pow(10, decimalLevel);
 ++decimalLevel;
 }
}
return value;

The first method is slightly faster according to my profiler.