tags:

views:

94

answers:

7

This is a general best practice question about creating parent / child relationships with objects.

Let's say I have Wheel and Car objects and I want to Add a Wheel object to car object

public class Car{

    private List<Wheel> wheels = new List<Wheel>();

    void AddWheel ( Wheel WheelToAdd)
        {
            wheels.Add(WheelToAdd)
            //Some Other logic relating to adding wheels here
        }
    }
}

So far so good. But what if I want to have a Car property of my wheel to say which parent car it relates to. Something like this

 public class Wheel {

     private Car parentCar;
     public Car 
     {
        get
        {
         return parentCar
        }

  }

}

When adding the wheel to the car, at which point would you set the parent property of the Wheel? You could set it in the Car.AddWheel method, but then the Car Property of the Wheel object would have to be Read/Write and then you could set it outside of the AddWheel method, creating an inconsistency.

Any thoughts, many thanks in advance

+1  A: 

You might want to consider having a setter that only sets Wheel.parentCar if that setting is null, but only if you're able to assume that your first setting will be valid and are thus able to disregard any other attempts.

Edit: but yes, that would be the proper place to add the Car object. You could also do a check such that you create (for example) Wheel.validateCar(Car carInQuestion), where it enforces that the parentCar property is only set where the current Wheel object exists in Car. This implies that you would have a public method for searching Car.wheels for membership based on a particular instance of a wheel. But that's only if you really feel the need to be strict.

mway
That's not a bad idea. But then it would kind of imply that you could change the Parent even when you couldn't
Mark 909
Not necessarily; you could throw an error or handle it however. The implementation and semantics are up to you, really, and how you want to have external objects interface with it - but using setters to conditionally assign properties is legitimate.
mway
Yes, that would protect it. But then if the wheel was removed from the Car then it would have no way of unsetting it.
Mark 909
By the same logic, you could use `Wheel.validateCar(Car carInQuestion)` to effectively handle setting and potentially unsetting it; it would obviously be a method that had access to the internal property, and thus invoking it with `Wheel.parentCar` as the argument, you could handle both operations depending on context.
mway
I will admit that this is probably not the best design choice, but if you HAVE to restrict access/visibility, you don't have very many __good__ choices without sacrificing either simplicity or security.
mway
A: 

It makes sense to me for the Car property of the Wheel object to be writable because there's nothing to stop you moving a Wheel from one ToyotaCamry (subclass of Car) to another. Though you might want to have a Drivable property on the original car to check that List.Count is still 4 before allowing Car.Drive() to be called.

Do you create Wheels with Cars? if so set the property at that time. Otherwise set it when Wheel is attached to Car.

Steve Townsend
Yes, you could move the wheel from one to another but then how would that car know that the wheel had been moved?
Mark 909
Each Car object needs some container to count the attached Wheels. See my Edit about checking Drivable-ity. Possibly more complex because back and front wheels may be different. Cars with 10 wheels, etc.
Steve Townsend
@Mark 909 - don't forget the spare
Steve Townsend
Yes, i need a collection within Car but it's the issue of keeping this collection and the parent property of the car in sync
Mark 909
@Mark 909 - the management of the collection and any ownership properties on the Part should be encapsulated in your Car `AddPart` and `RemovePart` methods, I would have thought
Steve Townsend
Agreed. But it's preventing the changing of this property by other code outside of the Car class.
Mark 909
Make sure Wheels can only be created with a clear sense of their ownership - don't allow your code's clients to randomly do `new Wheel()`. Then only allow the current owner (maybe that's a PartsFactory, maybe it's a PartsStore, maybe it's an AutoShop, maybe it's a Car) to have a reference to the Wheel. If anybody else wants a reference to the Wheel they have to take ownership first, and after that the Wheel is removed from the old owner, and hidden behind the new owner's interface.
Steve Townsend
+3  A: 

Bidirectional relationships tend to be very difficult to implement correctly. The prevailing advice here should be "don't do that, most likely you don't actually need it, and it will do you more harm than good."

If, after much careful consideration, you decide a bidirectional relationship is warranted, you can make the setter of the Car property internal, which doesn't fully protect against rogues setting unwanted values but it does limit the surface area significantly.

Bryan Watts
I'm coming to the same conclusion. Looking through the APIs for MS Word and MS Project, the parent / child releationships are available in all of these objects. Of course, many of the properties are Read Only to me as a user of the API but may not be internally.
Mark 909
Knowing a couple of people who worked on the original MSProject, I wouldn't necessarily assume its designs and object models represent "best practice". ;)
mikemanne
+4  A: 

A better design methodology, (Domain Driven Design) specifies that you should first decide what the domaim model requirements are for these entities... Not all entities need to be independantly accessible, and if Wheel falls into this category, every instance of it will always be a child member of a Car object and you don't need to put a Parent property on it... Car becomes what is referred to as a root entity, and the only way to access a Wheel is through a Car object.

Even when the Wheel object needs to be independantly accessible, the domain model requirements should tell you what the usage patterns require. Will any Wheel ever be passed around as a separate object, without it's parent ? In those cases is the Car parent relevant? If the identity of the parent Car is relevant to some piece of functionality, why aren't you simply passing the complete composite Car object to that method or module? Cases where a contained composite object (like a Wheel) must be passed on it's own, but the identity of the parent (the object it is part of) is needed and/or relevant, are in fact not a common scenario, and approaching your design using the above type of analysis can save you from adding unnessary code to you system.

Charles Bretana
In my specific scenario I have Passengers and Queues so a passenger could exist independently. maybe I should've used the concrete example - apologies.
Mark 909
I have properties of Passengers that are derived from their position in the Queue. e.g. Passenger.Position . To return this the Passenger would need to know the parent queue. Of course I could have a method on the Queue taking a parameter of Passenger and return the position but this seems messy also
Mark 909
When a piece of functionality requires access to two different objects, which are members of a heiarchy, deciding which type to put the functionality implementation into can be an art... The relative position of the two types is one, (but not necessarily the driving) factor in that decision.
Charles Bretana
Whichever one you put the implementaion into, an alternative implementation method can of course be put in the other which delegates to the actual implementaion in the first one.
Charles Bretana
A: 

Question: Can the Wheel instance be assigned to a different Car instance later in time?

1) If yes, the expose a public method to set the parentCar. You could make it a fluent method, for ease of code:

public class Wheel 
{
    private Car parentCar;

    public ParentCar 
    {
        get
        {
            return parentCar;
        }
    }

    public void SetParentCar(Car _parentCar)
    {
        parentCar = _parentCar;
        return this;
    }
}

Where you assign add the wheel, make the assignment:

void AddWheel ( Wheel WheelToAdd)
{
    wheels.Add(WheelToAdd.SetParentCar(this));
    //Some Other logic relating to adding wheels here
}



2) If not, just set the parent in the constructor.

public class Wheel 
{
    private Car parentCar;

    public ParentCar 
    {
        get
        {
            return parentCar;
        }
    }

    public Wheel(Car _parentCar)
    {
        parentCar = _parentCar;
    }
}
code4life
A: 

I'm going to go with "Don't". A wheel isn't just a property of a Car. A wheel could be used on a Trailer or a Cart or a FerrisWheel (ok, maybe that's a bit of a stretch). The point is, by making the car which uses the wheel a property of the wheel itself, you couple your wheel's design to being used on a car, and at that point, you lose any reusability for the class.

In this case, it seems like there's a lot to lose and little to gain by letting the wheel know what it's actually used on.

EricBoersma