C# Methods – Is It Okay for a Class to Use Its Own Public Method?

Instead of attempting to capture when control passes into your class and update the config then, explicitly update the configuration just before the I2CDevice is used.

Write a wrapper for I2CDevice which takes your DeviceConfig and I2CDevice as parameters to its constructor.

The wrapper can provide its own versions of the I2CDevice methods (looks like there are only 3), it would then ensure I2C configuration before executing the corresponding I2CDevice method.

You would then use this wrapper inside your Device1, Device2 classes in lieu of using I2CDevice directly.

public class I2CDeviceWrapper 
{
   private readonly I2CDevice _i2cdevice;
   private readonly DeviceConfig _config;

   public I2CDeviceWrapper(DeviceConfig config, I2CDevice device)
   {
       _config = config;
       _i2cdevice= device;
   }
   private void EnsureConfiguration()
   {
       //verify or set configuration so this wrapper's device is active.
   }
   public int Execute(I2CTransaction[] xActions, int timeout)
   {
       EnsureConfiguration();
       return _i2cdevice.Execute(xActions, timeout);
   }
   public I2CReadTransaction CreateReadTransaction (byte[] buffer)
   {
        EnsureConfiguration();
        return _i2cdevice.CreateReadTransaction(buffer);
   }
   public I2CWriteTransaction CreateWriteTransaction (byte[] buffer)
   {
        EnsureConfiguration();
        return _i2cdevice.CreateWriteTransaction(buffer);
   }

}

EDIT: if the I2CDevice is passed into the wrapper, the wrapper should not attempt to manage its lifecycle. I2CDevice provides a Dispose() method, in my experience (which does not include the micro framework) this indicates you risk a resource leak if it is not called. If this holds for the micro framework, you would want to provide one for the wrapper as well that calls the I2CDevice one.

Wrapper as Abstract Base:

If you use the wrapper as a base for each of your Device1, Device2 classes, the visibility of the Execute, I2CGetReadTransaction, I2CGetWriteTransaction methods and perhaps the _config field should be made protected.

public class Device1: I2CDeviceWrapper
{
    public Device1(DeviceConfig config, I2CDevice device):base(config, device)
    {}

    public something Method1()
    {
        ... 
        Execute(...);
        ...
    }        
}

Wrapper as member field: If the wrapper is a member of your Device1, Device2 classes, you would still pass your i2cBus as a parameter to those constructors, create A per Device wrapper using that devices' configuration and i2cBus and then use it going forward as you previously would have used i2cBuss.

public class Device1
{
    private readonly I2CDeviceWrapper _i2cBus;

    public Device1(I2CDevice i2cBus)
    {
         var config = //set up configuration
         _i2cBus = new I2CDeviceWrapper(config, i2cBus);
    }
    public void Method1()
    {
        ...
        _i2cBus.Execute(...);
        ...
    }
}

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.