Large file handling - Read algorithm breaks - C#

Answer 1

Your code is silently catching any exception that happens in the while loop and swallowing it.

This is a bad practice because it masks issues like the one you are running into.

Most likely, one of the methods you call inside the loop (int.Parse() for example) is throwing an exception because it encounters some problem in the format of the data (or your assumptions about that format).

Once an exception occurs, the loop that reads data is thrown off kilter because it is no longer positioned at a record boundary.

You should do several things to make this code more resilient:

1. Don't silently swallow exception in the run loop. Deal with them.
2. Don't read data byte by byte or field by field in the loop. Since your records are fixed size (77 bytes) - read an entire record into a byte[] and then process it from there. This will help ensure you are always reading at a record boundary.

Answer 2

• Don't put an empty generic catch block here and just silently catch and continue. You should check and see if you're getting an actual exception in there and go from there.
• There is no need for the byteToHexString function. Just use the 0x prefix before a hexadecimal number and it will do a binary comparison.

i.e.

if(al[0] == 0x16 && al[1] == 0x3C && al[2] == 0x02)
{
    ...
}

• I don't know what your doConvert function does (you didn't provide that source), but the BinaryReader class provides many different functions, one of which is ReadInt16. Unless your shorts are stored in an encoded format, that should be easier to use than doing your fairly obfuscated and confusing conversion. Even if they're encoded, it would still be far simpler to read the bytes in and manipulate them, rather than doing several roundtrips with converting to strings.

Edit

You appear to be making very liberal use of the LINQ extension methods (particularly ElementAt). Every time you call that function, it enumerates your list until it reaches that number. You'll have much better performing code (as well as less verbose) if you just use the built-in indexer on the list.

i.e. al[3] rather than al.ElementAt(3).

Also, you don't need to call Flush on an input Stream. Flush is used to tell the stream to write anything that it has in its write buffer to the underlying OS file handle. For an input stream it won't do anything.

I would suggest replacing your current sw.WriteLine call with this:

sw.WriteLine(BitConverter.ToString(packet)); and see if the data you're expecting on the row where it starts to mess up is actually what you're getting.

I would actually do this:

if (packet.Take(3).SequenceEqual(STARTCODE) &&
    packet.Skip(packet.Length - ENDCODE.Length).SequenceEqual(ENDCODE))
{
    ushort id = BitConverter.ToUInt16(packet, 3);
    ushort semistable = BitConverter.ToUInt16(packet, 5);
    byte contant = packet[7];

    for(int i = 8; i < 72; i += 2)
    {
        il.Add(BitConverter.ToUInt16(packet, i));
    }

    foreach(ushort element in il)
    {
        sw.WriteLine(string.Format("{0},{1},{2},{3}", id, semistable, constant, element);
    }

    il.Clear();
}
else
{
    //handle "bad" packets
}