Search code examples
c#eventsinterfaceclass-designcad

Best practice with a possibility of a null interface object within class


I've created a class that represent a component. This component has a width,height,x-Coordinate,y-Coordinate, etc. When I manipulate the width,height,x, and y, I want to keep the logic within the class. But there is an interface object within the Component Class that has similar values. This interface can be used to talk to different types of CAD software. The Shape interface can be null though.

So my question is what would be the best approach for this? In the example below, when I change "Y", should I check for null in the shape interface? Or maybe the Component Class has event handlers and the Shape Interface should register to them. So what would be best practice for designing this approach and what would give the best performance?

Appreciate it!

public class Component
{
    private double _y;

    public IShape Shape { get; set; }
    public string Name { get; set; }
    public double Width { get; set; }
    public double Height { get; set; }
    public double X { get; set; }

    public double Y
    {
        get => _y;

        set
        {
            _y = value;
            if (Shape != null) Shape.Y = value;
        }
    }

    public void Update_Shape()
    {
        //used to update the Shape Interface after it is assigned
    }

}

public interface IShape
{
    string Name { get; set; }
    double Width { get; set; }
    double Height { get; set; }
    double X { get; set; }
    double Y { get; set; }
}

UPDATE: To give more details, my interface will be able to talk to Microsoft Visio, and AutoCad. They are only meant to be used as a visual representation of the data, the are not in control of how many shapes, or where they are positioned. So in my application, the user can move, or change width/height within the application. If they have Visio open at the time, I want it to update Visio shapes as well. If it isn't open, then it doesn't matter(it will end up being updated later on). Same goes for AutoCad.


Solution

  • The best practice in this situation depends on what your design goals are.

    If you want to automatically update IShape and performance is critical then manually writing out your setters with a null check is going to give you both. Having an event that the IShape subscribes to causes you to have to invoke the event which is more expensive than checking null. And this has the benefit of keeping the mess inside the class as you only need to assign myComponent.X = 20;

    Having an event has it's benefits. If you look up the Observer Pattern you can find lots of good reads on this. If you have more than one IShape that would subscribe to your Component, say from both Visio and AutoCad at the same time this would be the way to go.

    Now in terms of performance, if you're update less than a few thousand components per second and you want cleaner code I would just call Update_Shape() when you want to synchronize the values. If you are assigning multiple values at the same time you can wrap them in an action that will automatically synchronize the values after it completes.

    var c = new Component();
    c.Shape = new Shape();
    
    c.UpdateShapes(s => {
        s.Height = 100;
        s.Width = 100;
        s.X = 5;
    });
    
    public class Component
    {
        public IShape Shape { get; set; }
        public string Name { get; set; }
        public double Width { get; set; }
        public double Height { get; set; }
        public double X { get; set; }
        public double Y { get; set; }
    
        public void UpdateShapes(Action<Component> update)
        {
            update(this);
            SyncronizeShapes();
        }
    
        public void SyncronizeShapes()
        {
            if (Shape != null)
            {
                Shape.Name = Name;
                Shape.Width = Width;
                Shape.Height = Height;
                Shape.X = X;
                Shape.Y = Y;
            }
        }
    }