views:

56

answers:

2

What is the best way to read an unsigned 24-bit integer from a C# stream using BinaryReader?

So far I used something like this:

private long ReadUInt24(this BinaryReader reader)
{
    try
    {
        return Math.Abs((reader.ReadByte() & 0xFF) * 256 * 256 + (reader.ReadByte() & 0xFF) * 256 + (reader.ReadByte() & 0xFF));
    }
    catch
    {
        return 0;
    }
}

Is there any better way to do this?

+7  A: 

Some quibbles with your code

  • You question and signature say unsigned but you return a signed value from the function
  • Byte in .Net is unsigned but you're using signed values for arithmetic forcing a later use of Math.Abs. Use all unsigned calculations to avoid this.
  • IMHO it's cleaner to shift bits using shift operators instead of multiplication.
  • Silently catching the exception is likely the wrong idea here.

I think it's more readable to do the following

private static uint ReadUInt24(this BinaryReader reader) {
    try {
        var b1 = reader.ReadByte();
        var b2 = reader.ReadByte();
        var b3 = reader.ReadByte();
        return 
            (((uint)b1) << 16) |
            (((uint)b2) << 8) |
            ((uint)b3);
    }
    catch {
        return 0u;
    }
}
JaredPar
Yes, more readable, but I wonder whether it is really a good idea to silently catch an exception here? (Of course, I don't know how the method is going to be used, but it doesn't leave a good feeling).
0xA3
@0xA3 agree it's likely the wrong idea and noted in my edit.
JaredPar
Thanks, but why is sliently catching the exception likely the wrong idea here?
TTT
@Alon because it treats error conditions, such as `IOExceptions`, no differently than valid reads of the value 0. The two are indistinguishable from the perspective of the caller. This may be your intent but it's not clear
JaredPar
A: 

This looks pretty elegant to me.

private static long ReadUInt24(this BinaryReader reader)
{
  try
  {
    byte[] buffer = new byte[4];
    reader.Read(buffer, 0, 3);
    return (long)BitConverter.ToUInt32(buffer, 0);
  }
  catch 
  { 
    // Swallowing the exception here might not be a good idea, but that is a different topic.
    return 0;
  }
}
Brian Gideon
The main reason I prefer JaredPar's answer is the explicit placement of the bytes. If, for example, the bytes were in a different order his solution is easy to fix, but this one would cause some problems trying to fix that up.
Dolphin
JaredPar's answer executes faster as well.
Brian Gideon