Using dynamic to avoid code duplication when wrapping an API

Question

I am dealing with 3 different types of cameras from the same manufacturer and want to write a wrapper for their API.

The API structure is like this:

Camera1 and Camera2 use the class ManagedCamera.

Camera3 uses the class ManagedGigECamera.

Both ManagedCamera and ManagedGigECamera inherit from an abstract class ManagedCameraBase.

The problem is that I can't just use polymorphism to deal with them because the methods for configuring these cameras are in their respective classes and not in the ManagedCameraBase. All other methods are in ManagedCameraBase.

My approach to this was creating an abstract class PointGreyCamera and two classes that inherit from it: PointGreyUSBFireWire and PointGreyGigE.

PointGreyCamera has two abstract methods, void SetConfigurations() and dynamic NewCamera() (and a lot of other methods that implement what I want to do with the cameras).

Since the children need to use methods from their respective classes to set configurations, PointGreyCamera has a protected dynamic camera variable.

This is part of the base class code:

public abstract class PointGreyCamera : ICamera
{
    // Dynamic because depending on the class that inherits it'll be a different type: ManagedCamera or ManagedGigeCamera
    protected dynamic camera;

    /// <summary>
    /// Initializes variables and camera via index.
    /// </summary>
    /// <param name="index">Camera to be initialized</param>
    protected PointGreyCamera(uint index)
    {
        isCapturing = false;
        camera = NewCamera();
        busMgr = new ManagedBusManager();

        InitializeCamera(index);
    }

    /// <summary>
    /// Initializes variables and camera via serial number.
    /// </summary>
    /// <param name="serialNumber">Camera serial</param>
    protected PointGreyCamera(string serialNumber)
    {
        isCapturing = false;
        camera = NewCamera();
        busMgr = new ManagedBusManager();

        InitializeCamera(serialNumber);
    }

    /// <summary>
    /// Set camera configurations and, if continuous mode is selected, starts capturing.
    /// </summary>
    /// <param name="mode">Continuous/Trigger</param>
    /// <param name="resolution">Width x Height</param>
    /// <param name="rotation">Rotation angle</param>
    /// <param name="flip">Flip enabled/disabled</param>
    public abstract void SetConfigurations(CameraMode mode, Resolution resolution, LmaRotationAngle rotation, LmaFlipMode flip);

    /// <summary>
    /// Initialize camera dynamically
    /// </summary>
    protected abstract dynamic NewCamera();

    // Examples of methods inside this class
    public byte[] GetImageData() { ... }
    public Bitmap GetImage() { ... }
    public Resolution GetResolution() { ... }
    public CameraInformation GetCameraInformation() { ... }
    public void StartCapture() { ... }
    public void StopCapture() { ... }
    public void SetFPS() { ... }
}

And one of the classes that inherit from it.

public class PointGreyGige : PointGreyCamera
{
    public PointGreyGige(uint index) : base(index) { }
    public PointGreyGige(string serialNumber) : base(serialNumber) { }

    protected override dynamic NewCamera()
    {
        return new ManagedGigECamera(); // ManagedGigECamera is a class from the API I'm trying to wrap
    }

    public override void SetConfigurations(CameraMode mode, Resolution resolution, LmaRotationAngle rotation, LmaFlipMode flip)
    {
        // The methods called here that don't appear are implemented in the base class
        StopCapture();

        SetImagingMode(resolution);
        SetImagingSettings();
        currentResolution = resolution;

        SetFPS();          

        rotationAngle = rotation;
        FlipImage(flip);

        ConfigureModeAndStartCapturing(mode);

        DisableAutomaticGainAndShutter();
    }

    private void SetImagingSettings()
    {
        GigEImageSettingsInfo settingsInfo = camera.GetGigEImageSettingsInfo();
        var camerasSettings = new GigEImageSettings()
        {
            offsetX = 0,
            offsetY = 0,
            width = settingsInfo.maxWidth,
            height = settingsInfo.maxHeight,
            pixelFormat = PixelFormat.PixelFormatMono8
        };
        camera.SetGigEImageSettings(camerasSettings);
    }

    private void SetImagingMode(Resolution resolution)
    {
        Mode format7Mode = Mode.Mode0;

        var maxResolution = new Resolution(camera.GetCameraInfo().sensorResolution);
        var secondResolution = new Resolution(maxResolution.Width / 2, maxResolution.Height / 2);


        if (resolution.Equals(maxResolution))
            format7Mode = Mode.Mode0;
        else if (resolution.Equals(secondResolution))
            format7Mode = Mode.Mode2;
        else
            throw new ArgumentException("Resolution is not supported.", nameof(resolution));

        camera.SetGigEImagingMode(format7Mode);
    }
}

The duplication I'm trying to avoid is of those public methods in PointGreyCamera, since they are the same for any kind of camera. But to have a base class, it seems I need this dynamic field.

I have a couple questions about this:

• This initialization is to avoid repeating the constructor in both child classes, since they are the same except for the class they use, but it does have a virtual member call in the constructor. Is this a good idea?

• Is this a good use of dynamic?

• How else could I deal with this without code duplication?

• With this approach, I need to check the camera type before initializing it. But, since I can check the type when initializing, I could have only one non-abstract Camera class that checks the type and configure the camera accordingly. This makes easier to initialize but does not use polymorphism. Is there some way to use polymorphism and still have a cleaner initialization?

Edit: Expanding on that last point of my question:

Currently, I need to do this:

// initialize to some of the base classes just to get type. It doesn't matter which because the code used is from the base class.
ICamera camera = new PointGreyGige(serial); 
var type = camera.GetCameraType();
if (type == CameraType.GigE)
    camera = new PointGreyGige(serial);
else
    camera = new PointGreyUSBFireWire(serial);

where in PointGreyCamera I have

public CameraType GetCameraType()
{
    var info = camera.GetCameraInfo(); //GetCameraInfo is a method from ManagedCameraBase
    return LmaConversion.ToCameraType(info.InterfaceType);
}

This seem weird. How can I avoid it?

Also, from that last piece of code, it seems possible to have just one class, say PointGreyCamera (non-abstract now), that checks the camera type internally and has different implementations of the SetConfiguration() for each type accordingly. While I don't like that approach, I also dislike having to check the camera type before properly initializing it. Any thoughts?

Answer 1

I'd say no, this is not a good use of dynamic. This looks like a problem that can be easily solved by generics.

public abstract class PointGreyCamera<TCamera> : ICamera where TCamera :  ManagedCameraBase
{
    protected TCamera Camera { get; set; }

    ...

    public void SetFPS() 
    { 
        //access some imaginary ManagedCameraBase.Fps property
        Camera.Fps = 60;
    }
}

public class PointGreyGige : PointGreyCamera<ManagedGigECamera>
{
    ... 

    private void SetImagingSettings()
    {
        ...
        //access ManagedGigECamera.SetGigEImageSettings method
        Camera.SetGigEImageSettings(camerasSettings);
    }
}

Also you should not call virtual or abstract methods in constructor.

Regarding your last question... The only point gray camera I have is this one, so I do not have a precise answer. :) If I understand your problem correctly, and if we remove your wrappers from the equation, your problem boils down to:

//pseudo code incoming
ManagedCameraBase CreateCamera(string serial)
{
     var camera =  new ManagedGigECamera();

     camera.Initialize(serial); //do some stuff to get correct info

     if (camera.GetCameraInfo() ...) // check type
     {
         return new ManagedGigECamera()
     }
     else
     { 
         return new ...; // another type of camera
     }
}

1. This does not look right to me. It is either one of the shittiest API's I have ever seen, or this API simply was not designed to be used as camera detection mechanism (the way you use it). I think the second option is more likely to be true.

2. When possible, I would prefer relying on correct configuration. I mean if you know the serial number of the camera beforehand you must know it's type as well. It is much easier to either let user select the type, or just load it form preset configuration file, for example:

   <HardwareConfiguration>
     <Camera Id="...">
        <SerialNumber>...</SerialNumber>
        <Type>...</Type>
        <PixelFormat>...</Type>
        <!-- etc... -->
     </Camera>
   </HardwareConfiguration>
   

   This approach scales really well, if later you decide to support cameras from other manufacturers.

3. If you have to auto-detect cameras and can't just get the type from settings, then I am not sure what is the best approach here. You should definitely try contacting Point Gray and ask them whether or not this is a correct way to manage their cameras. Most large manufacturers have fairly decent support staff.