4
\$\begingroup\$

Up-to-date Version of this question: Binary protocol variability V3.0

Summary of the problem: Parsing an incoming stream of events from a binary communication protocol, if we have some variations in devices to support and would not like to have one huge switch to include everything.

So, we use it this way:

    static void Main(string[] args)
    {
        using (var inputStream = FakeInputDataStream())
        using (var binaryReader = new BinaryReader(inputStream))
        using (var tokenizer = new DeviceTokenizer(binaryReader).AsLoggable())
        using (var reader = new EventReader(tokenizer))
            foreach (dynamic e in reader.ReadAll())
                Handle(e);
    }

    static void Handle(SessionStart e) =>
        Console.WriteLine("SessionStart, Hardware Version = " + e.Hardware);

    static void Handle(EnvironmentalReport e) =>
        Console.WriteLine("EnvironmentalReport, Temperature = " + e.Temperature);

Solution looks like this:

enter image description here

Company.Hardware

Base event class:

public abstract class Event
{
}

And EventReader:

public class EventReader : IDisposable
{
    public EventReader(ITokenizer tokenizer)
    {
        Tokenizer = tokenizer;
    }

    public void Dispose() => Tokenizer.Dispose();

    public IEnumerable<Event> ReadAll()
    {
        Event e = null;
        while (TryRead(out e))
            yield return e;
    }

    bool TryRead(out Event e)
    {
        try
        {
            var type = Tokenizer.Read<Type>("discriminator");
            e = (Event)Activator.CreateInstance(type, Tokenizer);
            return true;
        }
        catch (ObjectDisposedException)
        {
            e = null;
            return false;
        }
    }

    ITokenizer Tokenizer { get; }
}

Where:

public interface ITokenizer : IDisposable
{        
    T Read<T>(string token);
}

With base implementation:

public abstract class Tokenizer : ITokenizer,
    IRead<Type>
{
    protected Tokenizer(BinaryReader reader)
    {
        Reader = reader;
    }

    public void Dispose() => Reader.Dispose();

    public T Read<T>(string token)
    {
        if (this is IRead<T>)
            return (this as IRead<T>).Read();
        else
            throw new NotSupportedException();
    }

    Type IRead<Type>.Read() => Protocol[Reader.ReadInt32()];
    protected virtual Protocol Protocol => Protocol.Undefined;
    protected BinaryReader Reader { get; }
}

public interface IRead<T>
{
    T Read();
}

The idea is to allow processing of additional tokens by implementing IRead<T> on the subtype.

Protocol class here maps discriminators to Types and uses System.Collections.Immutable from NuGet:

public class Protocol
{
    public static readonly Protocol Undefined = 
        new Protocol(ImmutableDictionary<int, Type>.Empty);

    Protocol(IImmutableDictionary<int, Type> types)
    {
        Types = types;
    }

    public Protocol Support<TEvent>(int discriminator)
        where TEvent : Event =>
        new Protocol(Types.Add(discriminator, typeof(TEvent)));

    public Type this[int discriminator]
    {
        get
        {
            if (Types.ContainsKey(discriminator))
                return Types[discriminator];
            else
                throw new NotSupportedException();
        }
    }

    IImmutableDictionary<int, Type> Types { get; }
} 

As for the logging:

public interface ILog : IDisposable
{
    void Write(string line);
}

With one implementation:

class TextLog : ILog
{
    public TextLog(TextWriter writer)
    {
        Writer = writer;
    }

    public void Dispose() =>
        Writer.Dispose();

    public void Write(string line) =>
        Writer.WriteLine(line);

    TextWriter Writer { get; }
}

And consumer:

class LoggingTokenizer : ITokenizer
{
    public LoggingTokenizer(ITokenizer parent, ILog log)
    {
        Parent = parent;
        Log = log;
    }

    public void Dispose()
    {
        Parent.Dispose();
        Log.Dispose();
    }

    public T Read<T>(string name)
    {
        var value = Parent.Read<T>(name);
        Log.Write($"{name}={value}");
        return value;
    }

    ITokenizer Parent { get; }
    ILog Log { get; }
}

We would use it through:

public static class TokenizerLogging
{
    public static ITokenizer AsLoggable(this ITokenizer tokenizer) =>
        tokenizer.AsLoggable(Console.Out);

    public static ITokenizer AsLoggable(this ITokenizer tokenizer, TextWriter writer) =>
        tokenizer.AsLoggable(new TextLog(writer));

    public static ITokenizer AsLoggable(this ITokenizer tokenizer, ILog log) =>
        new LoggingTokenizer(tokenizer, log);
}

Company.Hardware.Controller

Let’s say we have an updated tokenizer:

public class ControllerTokenizer : Tokenizer,
    IRead<DateTime>, IRead<Version>
{
    public ControllerTokenizer(BinaryReader reader) 
        : base(reader)
    {
    }

    protected override Protocol Protocol => base.Protocol
        .Support<SessionStart>(1);

    DateTime IRead<DateTime>.Read() => new DateTime(Reader.ReadInt32());
    Version IRead<Version>.Read() => new Version(Reader.ReadInt32(), Reader.ReadInt32());
}

And an event:

public class SessionStart : Event
{
    public SessionStart(ITokenizer tokenizer)
        : this(
              tokenizer.Read<Version>("hardware-version"),
              tokenizer.Read<Version>("firmware-version"),
              tokenizer.Read<DateTime>("clock-time"))
    {
    }

    public SessionStart(Version hardware, Version firmware, DateTime clock)
    {
        Hardware = hardware;
        Firmware = firmware;
        Clock = clock;
    }

    public Version Hardware { get; }
    public Version Firmware { get; }
    public DateTime Clock { get; }
}

Company.Hardware.Controller.Device

EnvironmentalReport event:

public class EnvironmentalReport : Event
{
    public EnvironmentalReport(ITokenizer tokenizer)
        :this(
             tokenizer.Read<float>("temperature"),
             tokenizer.Read<float>("humidity"))
    {
    }

    public EnvironmentalReport(float temperature, float humidity)
    {
        Temperature = temperature;
        Humidity = humidity;
    }

    // actually, I use non-primitive types
    public float Temperature { get; } 
    public float Humidity { get; }
}

Which requires:

public class DeviceTokenizer : ControllerTokenizer,
    IRead<float>
{
    public DeviceTokenizer(BinaryReader reader) 
        : base(reader)
    {
    }

    protected override Protocol Protocol => base.Protocol
        .Support<EnvironmentalReport>(2);

    float IRead<float>.Read() => Reader.ReadSingle();
}

P.S. Parameter validation is omitted for brevity.

Big/Little endian handling:

I am planning to define this helper class:

public static class BigEndian
{
    public static int ReadInt32BE(this BinaryReader reader) =>
        BitConverter.ToInt32(reader.ReadValue(4), 0);

    public static uint ReadUInt32BE(this BinaryReader reader) =>
        BitConverter.ToUInt32(reader.ReadValue(4), 0);

    public static short ReadInt16BE(this BinaryReader reader) =>
        BitConverter.ToInt16(reader.ReadValue(2), 0);

    public static ushort ReadUInt16BE(this BinaryReader reader) =>
        BitConverter.ToUInt16(reader.ReadValue(2), 0);

    public static long ReadInt64BE(this BinaryReader reader) =>
        BitConverter.ToInt64(reader.ReadValue(8), 0);

    static byte[] ReadValue(this BinaryReader reader, int size)
    {
        var bytes = reader.ReadBytes(size);
        if (BitConverter.IsLittleEndian)
            Array.Reverse(bytes);

        return bytes;
    }
}

And at the controller level of tokenizer inheritance it will look like the following:

// big-endian controller
public abstract class FreescaleTokenizer : Tokenizer,
    IRead<Type>, IRead<int>, IRead<uint>, IRead<ErrorCode>
{
    protected FreescaleTokenizer(BinaryReader reader) 
        : base(reader)
    {
    }

    Type IRead<Type>.Read() => Protocol[Reader.ReadInt32BE()];
    uint IRead<uint>.Read() => Reader.ReadUInt32BE();
    int IRead<int>.Read() => Reader.ReadInt32BE();
    ErrorCode IRead<ErrorCode>.Read() => (ErrorCode)Reader.ReadInt32BE();
}

public enum ErrorCode: int
{
    OK = 0,
    FailedGoingCanada = 1
}

Are there any potential troubles?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ If you have various devices, any chance you'll get a different endianness? If there's any chance of that, create your own class and override the read methods for the reader so you can switch endianness as needed. I ran into that once. \$\endgroup\$
    – Peter4499
    Jun 8, 2016 at 2:43
  • \$\begingroup\$ @Peter4499 Thanks! Good point, overridable strategy property on tokenizer would help as I have a tokenizer specialization per concrete device. Could you please put your comment as an answer below to let me upvote? \$\endgroup\$ Jun 8, 2016 at 2:47

2 Answers 2

3
\$\begingroup\$

If you have various devices, any chance you'll get a different endianness? If there's any chance of that, create your own class and override the read methods for the reader so you can switch endianness as needed. I ran into that once.

Overall though, depending on the variability of data to expect, I find a lot of the classes are very coupled. You should consider creating interfaces for things like "ReadSingle" and passing those in. I've worked with telco devices (old school stuff) and we used to have to handle all sorts of weird permutations. The looser you couple the classes the less switch statements you'll end up with.

\$\endgroup\$
3
  • \$\begingroup\$ I have appended an *endian handling section to my question. Do you mind to have a look? Will it be enough to have anything like this? \$\endgroup\$ Jun 8, 2016 at 6:37
  • 1
    \$\begingroup\$ @DmitryNogin yes that's better. By decoupling I meant to pass interfaces but that works just as well (I'm very biased towards having interfaces everywhere - I'm one of those developers). The only other thing to handle is the 3 different way for strings to be stored in binary. 1) Null terminated 2) Length at the beginning of the frame 3) record length set. :) Sorry for the curve ball but at least it's something to keep in mind. \$\endgroup\$
    – Peter4499
    Jun 8, 2016 at 15:47
  • \$\begingroup\$ Thanks! Such things are usually easy to define and reuse as an extension methods to BinaryReader also. Probably I could come to V3.0 letting tokenizer deal with just logging after selection from the extended BinaryReader API... \$\endgroup\$ Jun 8, 2016 at 16:01
2
\$\begingroup\$

EventReader

A method following the TryXXX pattern should never throw an exception. So if either Tokenizer is null or Activator.CreateInstance() fails, this method will throw.

Protocol

Disclaimer: Not sure if the ImmutableDictionary behaves the same like a Dictionary<TKey,TValue>.

If you need to know if a key is contained in the dictionary and if yes use the Item getter of the dictionary you should use TryGetValue() instead.

From my answer here: https://codereview.stackexchange.com/a/110878/29371

Whenever you also need the value for a given key of a Dictionary you shouldn't use ContainsKey() together with the Item getter but you should use TryGetValue().

Internally these three methods are calling the FindEntry() method to check wether a given key exists. So calling this method only once through the TryGetValue() method should be the way to go.

\$\endgroup\$
1
  • \$\begingroup\$ 1) From TryXXX definition: When using this pattern, it is important to define the try functionality in strict terms. If the member fails for any reason other than the well-defined try, the member must still throw a corresponding exception. 2) Agree, thanks. \$\endgroup\$ Jun 8, 2016 at 6:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.