Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Consider the following simple relationship.

Class Diagram

Code

[DataContract(Name = "Wheel")]
public class Wheel { }

[DataContract(Name = "OffRoadWheel")]
public class OffRoadWheel : Wheel { }

[DataContract(Name = "RoadVehicle")]
public class RoadVehicle
{
    public Collection<Wheel> Wheels { get; private set; }

    public RoadVehicle()
    {
        Wheels = new Collection<Wheel>();
    }
}

[DataContract(Name = "Truck")]
public class Truck : RoadVehicle
{
    public Collection<OffRoadWheel> OffRoadWheels { get; private set; }

    public Truck()
    {
        OffRoadWheels = new Collection<OffRoadWheel>();
    }
}

The Collections are configured as Read-Only and initiated in the class constructor as per FXCop recommendation CAS2227. http://msdn.microsoft.com/en-us/library/ms182327(VS.80).aspx

Now I know all the rules about Covariance & Contravariance, and the varying capabilities of using Collections vs. Lists vs. Enumerables (vs. their Interface Equivalents). However the above object model creates a number of competing difficulties.

If you create a truck with 4 Off-Road Wheels, down cast it to a RoadVehicle, and then iterate the Wheels collection... it's empty.

One way around this would be to create a public new Collection<Wheel> Wheels property on the Truck class which on-the-fly casts the contents of the OffRoadWheel collection, but this breaks behaviour if you go to add or remove on that collection.

Another problem is serialization, you can alter the above to store and duplicate the inner Wheels and OffRoadWheels properties, but then you'd end up complete serializing both if you wanted to serialize Truck

Any other suggestions on how to make this more Developer Friendly object model where things happen as you'd expect them to?

share|improve this question
1  
I'm a little concerned about having classes to represent off road wheels and wheels. Other than attributes (tread pattern, size, width, max speed, etc.) are there any differences between the two? I think the design is flawed. –  Jeff Vanzella Nov 30 '12 at 17:15
    
it's a hacked together example to represent a real-world solution. –  Eoin Campbell Nov 30 '12 at 17:47
    
or do you think that kind of "Square Relationship" is inherently flawed in any system. –  Eoin Campbell Nov 30 '12 at 17:47
    
I think you could have just a Wheel class that has an attribute type (Off-Road, Touring, Racing, Slick, ...) in it. This will save you a bunch of extra classes, and the type could be an enum. –  Jeff Vanzella Nov 30 '12 at 18:05
add comment

2 Answers 2

I think the model here is broken. If you have a RoadVehicle that's actually Truck, the interface lets you to add a normal Wheel to it, which is wrong.

One way to make it work is to make the collection immutable (e.g. IEnumerable<T>). This can be a reasonable solution if you're okay with setting the wheels only in the constructor.

share|improve this answer
    
Alternatively rather than via the constructor you could have an AddWheel(Wheel wheel) method and the Truck could be responsible for knowing what wheels it supports? I guess this then could potentially lead to a builder pattern? i.e. AddWheel().AddDoors() .... –  dreza Nov 30 '12 at 22:30
    
@dreza I think that still breaks LSP. If you have a method AddWheel(Wheel), then it shouldn't accept only a specific type of wheel. –  svick Dec 1 '12 at 12:20
    
Wouldn't constructors get a bit crowded if this were the case. What would be another alternative to constructor injection? –  dreza Dec 1 '12 at 18:12
add comment

Create constructors that take appropriate wheel collections. then RoadVehicle constructor looks like this public RoadVehicle(Collection<OffRoadWheel> hotWheels) : base(hotWheels) {}

share|improve this answer
1  
That creates bi-directional coupling. The base types shouldn't know about the child types. e.g. what if the child types were defined in a seperate assembly, you'd end up with a cyclic reference to achieve what you're trying. –  Eoin Campbell Nov 30 '12 at 14:42
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.