2

Background

We (my coworkers and I) are architecting a high level C# API to interact with a hardware device's native Windows DLL.

First I will discuss the hardware-architecture, then get to the software-architecture part.

Main Device

The device has the following components:

  • Exactly 10 receive channels

  • Exactly 4 transmission channels

Receive Channels

There can only the exact specified number of receive channels on the device per the hardware specification (no more no less). The receive channels keep their own buffers as well as various getters/setters for their internal hardware configurations.

I have decided to declare the channels as a public 10 element array inside the "main device" class, for the following reasons:

  • Whenever new data is received we check the addresses on all channels in the "main device" to know which channel the received data is intended for. This is currently done in a foreach loop which is nice and clean.
  • When assigning the properties of the channels I can do so easily by using a foreach loop (again, provides cleanness).
  • Need to access the actual object, not just return an instance of the object.

Transmit Channels

I have decided to declare the channels as a public 4 element array inside the "main device" class, for the same reasons as the receive channels.

Problem

  1. The channels are publicly exposed.
  2. As one of my colleagues has asked, "what if someone enters an invalid index into the array?"

Question

Is the array the proper way of going about this? Is there another way? I am looking for some suggestions on the general architecture.

Reminder: I need to access the channel objects directly, however we go about it.

  • 1
    "Need to access the actual object, not just return an instance of the object." Why? – Kasey Speakman Mar 30 '16 at 15:10
  • @KaseySpeakman I need to change the state of those objects. – Snoop Mar 30 '16 at 15:12
  • I'm still confused as to why. I'm assuming you don't want to change the state of the object on the receive channel, because then that's not the actual data that was received anymore. But passing a reference to the receive channel's objects will do exactly that. Semantically, it's no longer a receive channel. – Kasey Speakman Mar 30 '16 at 15:22
  • @KaseySpeakman actually you don't want to assume that. There are properties of each of my 10 receive channels that need to be configured independently. I state this in my question, in the first paragraph under (Receive Channels). "various getters/setters for internal hardware configurations". – Snoop Mar 30 '16 at 15:24
  • @KaseySpeakman let me shed some more light on this... – Snoop Mar 30 '16 at 15:24
3
+50

I believe you should keep the amount of channels abstracted:

  • The amount of channels may vary in the future, e.g. hardware up/downgrade (new devices)

  • You may want to reserve some channels in the future, e.g. for out-of-band communications and notifications

The exactness should be kept and ensured in initialization.

You should expose the channels with the least specific type possible, such as IEnumerable<ReceivingChannel> and IEnumerable<TransmittingChannel>.

The MainDevice should expose a way to initialize or manage these collections, you have several non-exclusive options:

  • Constructor parameters
private List<IReceivingChannel> receivingChannels = new List<IReceivingChannel>();

private List<IReceivingChannel> transmittingChannels = new List<ITransmittingChannel>();

public MainDevice(IEnumerable<IReceivingChannel> receivingChannels,
                  IEnumerable<ITransmittingChannel> transmittingChannels)
{
    this.receivingChannels.AddRange(receivingChannels);
    this.transmittingChannels.AddRange(transmittingChannels);
}
  • Fluid initialization methods
public MainDevice WithReceivingChannels(IEnumerable<IReceivingChannel> receivingChannels)
{
    this.receivingChannels.AddRange(receivingChannels);
    return this;
}

public MainDevice WithReceivingChannels(params IReceivingChannel[] receivingChannels)
{
    return WithReceivingChannel((IEnumerable<IReceivingChannel>)receivingChannels);
}

public MainDevice WithTransmittingChannels(IEnumerable<ITransmittingChannel> transmittingChannels)
{
    this.transmittingChannels.AddRange(transmittingChannels);
    return this;
}

public MainDevice WithTransmittingChannels(params ITransmittingChannel[] transmittingChannels)
{
    return WithTransmittingChannels((IEnumerable<ITransmittingChannel>)transmittingChannels);
}

Perhaps throw an exception (e.g. InvalidOperationException) in case you use these methods after the object has been first used by actual worker methods. Or better yet, have these methods in a builder class, where you finally call GetDevice which will create and initialize the device, possibly with dependency injection (I will not discuss DI here.)

  • Methods

    AddReceivingChannel, RemoveReceivingChannel, AddTransmittingChannel, RemoveTransmittingChannel

    GetReceivingChannels returning IEnumerable<IReceivingChannel>, or a similar ReceivingChannels property

    GetTransmittingChannels returning IEnumerable<ITransmittingChannel>, or a similar TransmittingChannels property

    GetReceivingChannelById, GetReceivingChannelByAddress, GetTransmittingChannelById, GetTransmittingChannelByAddress

Don't use settable properties, as that is asking for trouble. The MainDevice should be the only entity responsible for managing its actual composition.

This way, you may start by using arrays or lists to store the channels, but switch to dictionaries or trees (e.g. SortedDictionary<TKey, TValue>) if accessing them by some key becomes a bottleneck. And the users of MainDevice are just as happy.

You can define a base interface IChannel for the common things that all channels do, such as opening and/or closing with an optional timeout, and have IReceivingChannel and ITransmittingChannel hold the specific operations, such as receiving and transmitting with an optional timeout:

public interface IChannel
{
    void Open(TimeSpan timeout);
    void Close(TimeSpan timeout);
}

// I'm using generics, but you don't have to if you'll always
// deal with the same data type, e.g. IEnumerable<byte>
//
// I'm also over-simplifying by not stating end-of-transmission
// and other usual channel state.

public interface IReceivingChannel<out T> : IChannel
{
    T Receive(TimeSpan timeout);
}

public interface ITransmittingChannel<in T> : IChannel
{
    void Transmit(T obj, TimeSpan timeout);
}

In case you need to access some inner object from a channel, define specific interfaces on which you may use the as operator instead of the actual class. For instance, a IFrobberContainer interface with a GetNativeFrobber method (or similar NativeFrobber property), which does not extend IReceivingChannel.

This way, any object can contain a frobber:

public interface IFrobberContainer
{
    IFrobber NativeFrobber { get; }
}

internal class FrobberReceivingChannel : IReceivingChannel<Thing>, IFrobberContainer
{
    public void Open(TimeSpan timeout) { /* ... */ }

    public void Close(TimeSpan timeout) { /* ... */ }

    public Thing Receive(TimeSpan timeout) { /* ... */ }

    public IFrobber NativeFrobber { get; private set; }
}

internal class FrobberTransmittingChannel : ITransmittingChannel<Thing>, IFrobberContainer
{
    public void Open(TimeSpan timeout) { /* ... */ }

    public void Close(TimeSpan timeout) { /* ... */ }

    public void Transmit(Thing obj, TimeSpan timeout) { /* ... */ }

    public IFrobber NativeFrobber { get; private set; }
}

In case you have something extra to do with frobber channels, you can ask if the channel has a frobber using the as operator.

With the new C# 6 null-conditional operator, you can do this:

(channel as IFrobberContainer)?.NativeFrobber?.Frobulate();

instead of this:

var frobberContainer = channel as IFrobberContainer;
if (frobberContainer != null)
{
    var frobber = frobberContainer.NativeFrobber;
    if (frobber != null)
    {
        frobber.Frobulate();
    }
}

By segregating these distinct aspects into interfaces, you make your actual objects more flexible, more easily replaceable and mockable (good for unit testing).


If you insist on having 10 receiving channels and 4 transmitting channels, then you probably shouldn't use collections, but conversely use very good names for 14 properties. For generic iteration, you can keep internal collections initialized once, to avoid obvious code repetition.

You should still use interfaces and document which ones each channel implements, instead of using actual classes as the types for these properties, to retain the replaceability benefits for refactoring, maintenance and testing purposes.

  • 1
    Wow, thank you for taking the time in putting this together. – Snoop Mar 30 '16 at 16:02
1

Since this is a hardware device, I would not worry too much about being able to dynamically add or remove channels, rather I would keep it simple stupid. As a consumer of the device interface i would better off accessing it like this,

var device = //initialize the device

device.In1.Write(dataIn1);
device.In2.Write(dataIn2);

var out = device.Out1.Read();

so the channel interface would look like follows,

IWriteChannel{
   bool Write(object data);
}

IReadChannel{
   object Read();
}

This is pretty simple interface, and if you want you can add types to the interface as well. For example

IWriteChannel{
   bool Write<T>(T data);
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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