views:

74

answers:

2

I'm writing a plug-in for a 3D modeling program. I have a custom class that wraps instances of elements in the 3D model, and in turn derives it's properties from the element it wraps. When the element in the model changes I want my class(es) to update their properties based on the new geometry.

In the simplified example below. I have classes AbsCurveBasd, Extrusion, and Shell which are all derived from one another. Each of these classes implement a RefreshFromBaseShape() method which updates specific properties based on the current baseShape the class is wrapping.

I can call base.RefreshFromBaseShape() in each implementation of RefreshFromBaseShape() to ensure that all the properties are updated. But I'm wondering if there is a better way where I don't have to remember to do this in every implementation of RefershFromBaseShape()? For example because AbsCurveBased does not have a parameterless constructor the code wont even compile unless the constructors call the base class constructors.

public abstract class AbsCurveBased
{
    internal Curve baseShape;
    double Area{get;set;}

    public AbsCurveBased(Curve baseShape)
    {
        this.baseShape = baseShape;
        RefreshFromBaseShape();
    }

    public virtual void RefreshFromBaseShape()
    {
        //sets the Area property from the baseShape
    }
}


public class Extrusion : AbsCurveBased
{
    double Volume{get;set;}
    double Height{get;set;}

    public Extrusion(Curve baseShape):base(baseShape)
    {
        this.baseShape = baseShape;
        RefreshFromBaseShape();
    }

    public override void RefreshFromBaseShape()
    {
        base.RefreshFromBaseShape();
        //sets the Volume property based on the area and the height
    }
}


public class Shell : Extrusion
{
    double ShellVolume{get;set;}
    double ShellThickness{get;set;}

    public Shell(Curve baseShape): base(baseShape)
    {
        this.baseShape = baseShape;
        RefreshFromBaseShape();
    }

    public void RefreshFromBaseShape()
    {
        base.RefreshFromBaseShape();
        //sets this Shell Volume from the Extrusion properties and ShellThickness property
    }
}
A: 

The way you do it is a very typical pattern, and there's certainly nothing wrong with it - it's how such a thing is usually done. There's no way to force that base call, on presumption that when and where it happens is always up to the implementer. If you are really keen on forcing it, then you should consider using a protected event instead:

public abstract class AbsCurveBased
{
    internal Curve baseShape;
    double Area{get;set;}

    public AbsCurveBased(Curve baseShape)
    {
        this.baseShape = baseShape;
        RefreshFromBaseShape();
    }

    public void RefreshFromBaseShape()
    {
        //sets the Area property from the baseShape
        ...

        // call child handlers
        var handler = RefreshingFromBaseShape;
        if (handler != null)
            handler();
    }

    protected event Action RefreshingFromBaseShape;
}

public class Shell : Extrusion
{
    double ShellVolume{get;set;}
    double ShellThickness{get;set;}

    public Shell(Curve baseShape): base(baseShape)
    {
        this.RefreshingFromBaseShape += RefreshingFromBaseShapeHandler;

        this.baseShape = baseShape;
        RefreshFromBaseShape();
    }

    private void RefreshingFromBaseShapeHandler()
    {
        //sets this Shell Volume from the Extrusion properties and ShellThickness property
    }
}

That way any class in the inheritance chain only has control over its handler(s), and cannot unregister the handlers of his ancestors, or insert them into the chain before the ancestors insert theirs.

For your particular case, though, this seems like too much complexity for no worth.

Also, if RefreshFromBaseShape is only ever called from constructor, then perhaps it should rather be a parameter of that constructor. Consider:

public abstract class AbsCurveBased
{
    internal Curve baseShape;
    double Area{get;set;}

    public AbsCurveBased(Curve baseShape)
    {
        this.baseShape = baseShape;

        //sets the Area property from the baseShape
    }

    protected AbsCurveBased(Curve baseShape, Action refreshFromBaseShape):
        this(baseShape)
    {
        refreshFromBaseShape();
    }
}

public class Shell : Extrusion
{
    double ShellVolume{get;set;}
    double ShellThickness{get;set;}

    public Shell(Curve baseShape):
        base(baseShape, RefreshFromBaseShape)
    {
    }

    protected Shell(Curve baseShape, Action refreshFromBaseShape):
        this(baseShape)
    {
        refreshFromBaseShape();
    }

    private void RefreshFromBaseShape()
    {
        //sets this Shell Volume from the Extrusion properties and ShellThickness property
    }
}
Pavel Minaev
Ahh yeah I was thinking there was something I could do involving events or delgates. But that doesn't make things any simpler that just using base.RefreshFromBaseShape()
Eric Anastas
It doesn't make it simpler. It just provides the guarantee that no derived class will be able to skip the code provided by his ancestor (though it can still cut off any of its own descendants).
Pavel Minaev
A: 

I'm not sure why you'd want a separate virtual method to do this. Why can't each class do its calculation in its own constructor? In your example, Area would be calculated in the AbsCurveBased constructor; Volume & Height in the Extrusion constructor etc.

In general it's bad practice to call a virtual function from a constructor, since the virtual function is called in the sub-classes before their constructors have finished. So the method in the subclass is running while the object is only partly through construction.

Update after comment

In this case I would have a private DoCalculate() method in each of the classes. This would be called by the constructors and also the RefreshFromBaseShape() which would no longer be called in the constructors. It doesn't alleviate the need to chain your base calls though, which is a normal pattern.

Jon
I need to calculate the properties when the object is instantiated, but potentialy also later if the base shape ever changes.
Eric Anastas