C#: How would you suggest to refactor these classes? (Interfaces, Aggregation or anything else)...

Question

Having the following classes, in two different assemblies:

class Member
{
    public string Label {get;set;}
    // Lots of other fields...

    public double Thickness {get;set;}
    public double Width {get;set;}
    public double Length {get;set;}
    public double GetVolume ()
    {
        return Thickness * Width * Length;
    }

    // Other methods
}

class OtherMember
{
    public string CompositeLabel {get;set;}
    // Lots of other fields (not related to other class: Member)

    public double Thickness {get;set;}
    public double Width {get;set;}
    public double Length {get;set;}
    public double GetVolume ()
    {
        return Thickness * Width * Length;
    }

    // Other methods
}

I want to refactor the Thickness, Width, Length properties and the GetVolume method to another class (Dimensions?) as to DRY...

I could then have a field/property in those classes to give access to a Dimensions instance.

Is this a good way to go?

I've read succinctly about having interfaces instead of concrete classes, should I inherit from an interface?

Also, what about immutability?

I feel I'm often lost about getting my domain classes right. Does anyone have some advice on this?

Answer 1

I would solve this problem by creating an interface called IThreeDimensional with an extension method GetVolume(). I would do it like so:

interface IThreeDimensional
{
    double Thickness {get; set;}
    double Width {get; set;}
    double Length {get; set;}
}

static double GetVolume(this IThreeDimensional value)
{
    return value.Thickness * value.Width * value.Length;
}

Then you could make all your classes implement IThreeDimensional and then they would inherit GetVolume() for free.

Answer 2

Since you have a lot of state and behavior in common I would recommend a base class. Try something like this:

class Member
{
    public string Label { get; set; }
    public double Thickness { get; set; }
    public double Width { get; set; }
    public double Length { get; set; }
    public double GetVolume()
    {
     return Thickness * Width * Length;
    }
}

class OtherMember : Member
{
    public string CompositeLabel { get; set; }
}

And depending on what the Label property is for you have the option of making it virtual and override the implementation in the derived class:

class Member
{
    public virtual string Label { get; set; }
    public double Thickness { get; set; }
    public double Width { get; set; }
    public double Length { get; set; }
    public double GetVolume()
    {
     return Thickness * Width * Length;
    }
}

class OtherMember : Member
{
    string label;

    public override string Label
    {
     get { return this.label; }
     set { this.label = value; }
    }
}

Answer 3

Containment or inheritance?

If both these things are dimensions, then the properties could be put into an superclass named Dimensions from which these things both inherit.

If both these things have dimensions, then the properties could be put into a class named Dimensions which these things could contain as a property / data member.

Other people have shown what inheritance would look like; this is what containment would look like:

class Dimensions
{
    public double Thickness {get;set;}
    public double Width {get;set;}
    public double Length {get;set;}
    public double GetVolume ()
    {
        return Thickness * Width * Length;
    }
}

class Member
{
    public string Label {get;set;}
    // Lots of other fields...
    public Dimensions Dimensions {get;set;}

    // Other methods
}

class OtherMember
{
    public string CompositeLabel {get;set;}
    // Lots of other fields (not related to other class: Member)

    public Dimensions Dimensions {get;set;}

    // Other methods
}

Some people say, as a design principle or rule-of-thumb, "prefer containment over inheritance"; see also Inheritance vs. Aggregation.

---

Interfaces instead of concrete classes?

Interfaces instead of concrete classes add an extra layer of indirection. They're better than a subclass in two ways:

• They're completely implementation-independent; I might want two implementations of something, one for real-life and one for unit-testing, which have the same interface but whose implementations having nothing in common which eah other.

• A class can implement more than one interface (but can only have one subclass). That's why IEnumerable and IDisposable are better as interfaces than as subclasses: because something might be IEnumerable, and IDisposable, and a subclass of something else.

Neither of these arguments are necessarily applicable to your circumstance.

---

What about immutability?

Immutability is especially important if the things are structs (not so important here where they're classes rather than structs).

Answer 4

Something that has thickness, width, length and volume sounds to me like a Solid. So let's call it that.

Is Member a Solid? Looks like it to me, but the classes would have to be more explicitly specified to be sure. Same goes for OtherMember. So you could simply have Member and OtherMember extend Solid as subclasses.

If you didn't want to go that route - maybe there is something more important to Member and OtherMember, some other class they should extend, or maybe there isn't a true "is-a" relationship with Solid - you could have them implement the ISolid interface, and delegate all ISolid methods to a Solid instance member. Generally, we prefer composition (the second approach described here) to inheritance.

Answer 5

I would do an interface for the properties and the method:

interface IThreeD
{
    double Thickness {get; set;}
    double Width {get; set;}
    double Length {get; set;}

    double GetVolume();
}

And then a base class that provides the common implementations. Something like:

class BaseMember
{
    public string Label { get; set; }
    public double Thickness { get; set; }
    public double Width { get; set; }
    public double Length { get; set; }
    public double GetVolume()
    {
        return Thickness * Width * Length;
    }
}

class Member : BaseMember
{ 
    // Things specific to Member
}

class OtherMember : BaseMember
{
    // Things specific to OtherMember
}

Answer 6

Something like this I believe is what you're looking for

interface I3d
{
    double Thickness {get; set;}
    double Width {get; set;}
    double Length {get; set;}
    double GetVolume
}


public class ThreeDimensionalShape : I3d
{
  public double Thickness {get; set;}
  public double Width {get; set;}
  public double Length {get; set;}
  public double GetVolume()
  {
      return this.Thickness * this.Width * this.Length;
  }
}

class Member : ThreeDimensionalShape
{
    public string Label {get;set;}
    // Lots of other fields...

    // Other methods
}

class OtherMember : ThreeDimensionalShape
{
    public string CompositeLabel {get;set;}
    // Lots of other fields (not related to other class: Member)

    // Other methods
}

Answer 7

I would agree that you should create a Dimensions class that has the Thickness, Length, Width properties, along with the GetVolume() method. You probably do NOT want to create a base class with these members and have Member and OtherMember inherit from it because Member and OtherMember have dimensions, they are not super classes of dimensions. I'll try to lay out some guidelines so you can make the best decision for your case below.

For the interface, it is possibly a good idea since there are multiple ways volume can be calculated.

public interface ISolid{
  double Volume { get; }
}

public class Cylinder: ISolid{
  public double Height { get; set; }
  public double Radius { get; set; }
  public double Volume {
    get
    {
      return (2 * Math.Pi * Radius ^ 2) * Height;
    }
  }
}

public class Cube: ISolid{
  public double Width { get; set; }
  public double Height { get; set; }
  public double Depth { get; set; }
  public double Volume {
    get
    {
      return Width * Height * Depth;
    }
  }
}

It isn't clear what Member and OtherMember are, they have labels and other things that may not be represented well by inheriting straight from these types defined above. Also, multiple inheritance can make things complicated. So you need to look at your own domain objects and ask yourself, is this class a superclass (inheritance) or does it just need to use the other class to define some characteristic of itself (composition)

public class SodaCan: Cylinder
{
  public SodaCan(string brand)
  {
    Brand = brand;
    IsFull = true;
    Height = 5D;
    Radius = 1.5D;
  }

  public string Brand { get; private set; }
  public bool IsFull { get; set; }
}

or

public class SodaCan: BeverageHolder
{
  public SodaCan(string brand)
  {
    Brand = brand;
    IsFull = true;
    Dimensions = new Cylinder { Height = 5D, Radius = 1.5D };
  }

  private ISolid Dimensions { get; set; }

  public double Volume {
    get {
      return Dimensions.Volume;
    }
  }
}

If all of your objects volumes can be found by using the Lenth * Height * Thickness, then you probably don't need the interface. Interfaces are good when you might have a bunch of objects with the same behavior, but the behavior is performed different. For example the volume of a cylinder vs. the volume of a cube. It doesn't hurt to create an interface anyway, but in this case you might not need it. In terms of should you create a base class, or a class to encapsulate the common behavior and use composition, that depends on your Domain. The soda can could be a cylinder, or it could be a BeverageHolder. A box of wine could also be a beverage holder, and those are not both Cylinders, so you have to look at what else you might want to inherit from in your domain.

Answer 8

I have an aversion to relying strictly on interface-based implementations. That aversion arises from the simple fact that interfaces are immutable. Once you deploy them, you're stuck with them.

Having said that, I also have an aversion to relying strictly on inheritance-based implementations. In .NET-land, we only get to choose one base class. Make sure it's a good one! (And make sure it's the right one.)

I think that the right thing to do is to define an interface for your dimensions. Then, define a class that implements the interface, providing the most common implementation of the GetVolume method, as others have suggested. Make sure that that method is overridable, and that consumers can derive from the class.

I'd wholeheartedly embrace the notion that the width, height, and depth of an object are attributes, and that they should be plugged into it through a property. Thus:

var dimensions = new Dimensions(x, y, z);
var some3dObject = new ThreeDObject();
some3dObject.Dimensions = dimensions;

makes perfect sense to me.

Interfaces vs. inheritance is a question of trade-offs. One way or the other, you're going to give up something: either the ability to extend the interface, or the ability to change your base class down the road. Which one do you feel more comfortable doing?

On the other hand, there are extension methods, so that could solve a whole lot of problems. Just take great care to make sure your code doesn't grow unmanageable over time.

Good luck!