VARIANT structure deserialization code organization

Question

In my project i work with COM object via System.Reflection. COM object returns pointer on structure VARIANT, that i cast as object and then i cast one as byte array. There are many structures that i get from COM object. Since i know about how organize these structure i write static class to deserialize them byte-by-byte according to their structure. Because i have many structures with different fields i have many methods: one method for deserialize one structure. Then i write generics method when i call these method depend on some number of structure. Is this correct code organization?

Class:

using SpRReaderSocketServerLib.SpRRecordLib;
using System;
using System.IO;
namespace SpRReaderSocketServerLib.SpRTools
{
 /// <summary>
 /// Static class for deserialize structures from unmanaged code via BinaryReader and MemoryStream.
 /// 1. SPR_MSG_RECORD_START => MSGRecordStart - ok
 /// 2. SPR_MSG_RECORD_STOP => MSGRecordStop - ok
 /// 3. SPR_MSG_ANI_DETECT => MSGPhoneNumber - ok
 /// 4. SPR_MSG_DIALLED_PHONE => MSGPhoneNumber
 /// 5. SPR_MSG_RING => MSGRing - ok
 /// 6. SPR_MSG_MISSED_PHONE => MSGMissedPhone - ok - for tests
 /// </summary>
 public static class Deserializer
 {
 /// <summary>
 /// Generic method for deserialize messages.
 /// </summary>
 /// <typeparam name="T">Struct</typeparam>
 /// <param name="msgId">Message Id</param>
 /// <param name="msgObject">Message as object</param>
 /// <returns>T Struct</returns>
 public static T DeserializeMsg<T>(int msgId, object msgObject) where T : struct
 {
 byte[] bytes = (byte[])msgObject;
 switch (msgId)
 {
 case SpRecordLibTypesW.SPR_MSG_HEADER:
 return (T)Convert.ChangeType(DeserializeHeader(bytes), typeof(T));
 case SpRecordLibTypesW.SPR_MSG_RING:
 return (T)Convert.ChangeType(DeserializeRing(bytes), typeof(T));
 case SpRecordLibTypesW.SPR_MSG_RECORD_START:
 return (T)Convert.ChangeType(DeserializeRecordStart(bytes), typeof(T));
 case SpRecordLibTypesW.SPR_MSG_RECORD_STOP:
 return (T)Convert.ChangeType(DeserializeRecordStop(bytes), typeof(T));
 case SpRecordLibTypesW.SPR_MSG_ANI_DETECT:
 case SpRecordLibTypesW.SPR_MSG_DIALLED_PHONE:
 return (T)Convert.ChangeType(DeserializePhoneNumber(bytes), typeof(T));
 case SpRecordLibTypesW.SPR_MSG_MISSED_PHONE:
 return (T)Convert.ChangeType(DeserializeMissedPhone(bytes), typeof(T));
 default:
 return default(T);
 }
 }
 /// <summary>
 /// Deserialize object with MSGHeader structure.
 /// </summary>
 /// <param name="message">Message from SpRecord system</param>
 /// <returns>MSGHeader</returns>
 private static MSGHeader DeserializeHeader(byte[] message)
 {
 Stream s = new MemoryStream(message);
 MSGHeader msgHeader = new MSGHeader();
 using (BinaryReader br = new BinaryReader(s))
 {
 msgHeader.dwMsgSize = br.ReadInt32(); // 4
 msgHeader.dwMsg = br.ReadInt32(); // + 4
 msgHeader.MsgTime = DateTime.FromOADate(br.ReadDouble()); // + 8
 }
 return msgHeader; // = 16
 }
 /// <summary>
 /// Deserialize object with MSGRing structure.
 /// </summary>
 /// <param name="message">Message from SpRecord system</param>
 /// <returns>MSGRing</returns>
 private static MSGRing DeserializeRing(byte[] message)
 {
 Stream s = new MemoryStream(message);
 MSGRing msgRing = new MSGRing();
 using (BinaryReader br = new BinaryReader(s))
 {
 msgRing.mh.dwMsgSize = br.ReadInt32(); // 4
 msgRing.mh.dwMsg = br.ReadInt32(); // + 4
 msgRing.mh.MsgTime = DateTime.FromOADate(br.ReadDouble()); // + 8
 msgRing.cbChannelName = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_CHANNEL_NAME_CHARS * 2)); // + 48*2
 msgRing.dwRingCount = br.ReadInt32(); // + 4
 }
 return msgRing; // 116
 }
 /// <summary>
 /// Deserialize object with MSGMissedPhone structure.
 /// </summary>
 /// <param name="message">Message from SpRecord system</param>
 /// <returns>MSGMissedPhone</returns>
 private static MSGMissedPhone DeserializeMissedPhone(byte[] message)
 {
 Stream s = new MemoryStream(message);
 MSGMissedPhone msgMissedPhone = new MSGMissedPhone();
 using (BinaryReader br = new BinaryReader(s))
 {
 msgMissedPhone.mh.dwMsgSize = br.ReadInt32(); // 4
 msgMissedPhone.mh.dwMsg = br.ReadInt32(); // + 4
 msgMissedPhone.mh.MsgTime = DateTime.FromOADate(br.ReadDouble()); // + 8
 msgMissedPhone.cbChannelName = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_CHANNEL_NAME_CHARS * 2)); // + 48*2
 msgMissedPhone.cbPhoneFrom = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_PHONE_NUMBER_CHARS * 2)); // + 48*2
 msgMissedPhone.cbSubPhoneFrom = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_SUBPHONE_NUMBER_CHARS * 2)); // + 24*2
 msgMissedPhone.cbPhoneTo = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_PHONE_NUMBER_CHARS * 2)); // + 48*2
 msgMissedPhone.cbSubPhoneTo = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_SUBPHONE_NUMBER_CHARS * 2)); // + 24*2
 }
 return msgMissedPhone; // = 400
 }
 /// <summary>
 /// Deserialize object with MSGRecordStart structure.
 /// </summary>
 /// <param name="message">Message from SpRecord system</param>
 /// <returns>MSGRecordStart</returns>
 private static MSGRecordStart DeserializeRecordStart(byte[] message)
 {
 Stream s = new MemoryStream(message);
 MSGRecordStart msgRecordStart = new MSGRecordStart();
 using (BinaryReader br = new BinaryReader(s))
 {
 msgRecordStart.mh.dwMsgSize = br.ReadInt32(); // 4
 msgRecordStart.mh.dwMsg = br.ReadInt32(); // + 4
 msgRecordStart.mh.MsgTime = DateTime.FromOADate(br.ReadDouble()); // + 8
 msgRecordStart.cbChannelName = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_CHANNEL_NAME_CHARS * 2)); // + 48*2
 msgRecordStart.dwRecordType = br.ReadInt32(); // + 4
 msgRecordStart.cbPhoneFrom = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_PHONE_NUMBER_CHARS * 2)); // + 48*2
 msgRecordStart.cbSubPhoneFrom = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_SUBPHONE_NUMBER_CHARS * 2)); // + 24*2
 msgRecordStart.cbPhoneTo = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_PHONE_NUMBER_CHARS * 2)); // + 48*2
 msgRecordStart.cbSubPhoneTo = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_SUBPHONE_NUMBER_CHARS * 2)); // + 24*2
 msgRecordStart.cbFileName = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRecordLibTypesW.MAX_PATH * 2)); // + 260*2
 }
 return msgRecordStart; // = 924
 }
 }
}

• svick: I think that if possible, you shouldn't do this manually. Can you use .Net 4.5.1? It seems to have a new method that could help with this.
• Konstantin: I use .NET 4. As to marshalling i have some problem with one, for example all structures are nested and before parse any outer structure i must parse inner structure in order to get type of outer structure. Deserializing via BinaryReader good option for me. Everything work fine. I would like to ask about my code.

Answer 1

int msgId

I think it would be better if this was an enum. That way, it's clear what values are acceptable for this parameter.

object msgObject

Since this method only accepts byte[], I think that should be the parameter. It will then be a responsibility of the caller to do the casting.

(T)Convert.ChangeType(DeserializeHeader(bytes), typeof(T))

Unless you actually need to perform some conversions here, what you can do is to write just (T)(object)DeserializeHeader(bytes). This works around the fact that the compiler doesn't know what code to generate for (T)DeserializeHeader(bytes), because it could be a custom conversion.

default:
 return default(T);

I think that this means that the msgId parameter is invalid, so this should be an error and you should throw an exception here.

Stream s = new MemoryStream(message);

This variable is very simple and used only in one place, you could inline it: new BinaryReader(new MemoryStream(message)).

msgRing.cbChannelName = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_CHANNEL_NAME_CHARS * 2));

Why is this a char[]? Using string would be more natural. To do that, just use GetString() instead of GetChars(). Though that seems to result in string that contains lots of null chars at the end. To fix that, you could use .TrimEnd('0円').

msgRing.mh.dwMsgSize = br.ReadInt32(); // 4
msgRing.mh.dwMsg = br.ReadInt32(); // + 4
msgRing.mh.MsgTime = DateTime.FromOADate(br.ReadDouble()); // + 8
msgRing.cbChannelName = System.Text.Encoding.Unicode.GetChars(br.ReadBytes(SpRRecordLib.SpRecordLibTypesW.MAX_CHANNEL_NAME_CHARS * 2)); // + 48*2
msgRing.dwRingCount = br.ReadInt32(); // + 4

You could use object initializer here, something like:

return new MSGRing
{
 mh =
 {
 dwMsgSize = br.ReadInt32(), // 4
 dwMsg = br.ReadInt32(), // +4
 MsgTime = DateTime.FromOADate(br.ReadDouble()) // +8
 },
 cbChannelName =
 Encoding.Unicode.GetString(
 br.ReadBytes(
 SpRRecordLib.SpRecordLibTypesW.MAX_CHANNEL_NAME_CHARS * 2)), // + 48*2
 dwRingCount = br.ReadInt32() // + 4
}; // 116

• Konstantin: Thank you, svick. I will take into account all of your arguments and fix your code!