views:

196

answers:

5

This may be a beginner question but is there a standard way to refactor the duplication of the Wheel property into the abstract class yet still maintain the explicit cast to the Part type. Let’s assume we have to prevent a FastCarWheel from being put on a SlowCar, and that there are many properties just like this one.

abstract class Car {}

class FastCar : Car
{
    public FastCarWheel Wheel { get; set; }
} 

class SlowCar : Car
{
    public SlowCarWheel Wheel { get; set; }
} 

abstract class WheelPart {}

class FastCarWheel: WheelPart {}

class SlowCarWheel: WheelPart {}

In this type of scenario is it common to just allow this type of duplication? I was thinking of making use of Generics but it just seems like I’m moving the issue around, and it gets worse for each additional property that behaves this way.

abstract class Car <P>
    where P : Part
{
    protected abstract P Wheel { get; set; }
}

Thanks

A: 

Define a wheel interface (IWheel):

public interface IWheel
{
}

Implement the interface for FastCarWheel and SlowCarWheel eg

public class FastCarWheel : IWheel
{
}

Now your abstract class becomes:

abstract class Car 
{
 public IWheel Wheel { get; set; }
}

Subclasses of Car are then free to use whatever implementation of Wheel they choose:

FastCar fastCar = new FastCar();
fastCar.Wheel = new FastCarWheel();
Winston Smith
That's the problem, though. He DOESN'T want any car to have any type of wheel. He wants FastCars to only have FastWheels, and SlowCars to only have SlowWheels. An interface would not accomplish this.
Joseph
If a FastCar cannot have a SlowCarWheel, then that logic belongs in FastCar, and not in the base class.
Winston Smith
I agree, but the problem is your code would promote the use of this (fastCar.Wheel = new SlowCarWheel();) which he explicitly said he didn't want to allow. I agree, though, that this logic does not belong in the base class.
Joseph
+1  A: 

I think using a Fast or Slow policy can help put the correct wheel for a given car type (where both Car and Wheel are dependent on the policy and a Car object has, say, a private aggregration of wheels).

dirkgently
+1  A: 

This solution isn't polymorphic, but might be your only option if you need visibility at the base class level:

abstract class Car
{
    private CarWheel wheel;
    public CarWheel Wheel
    {
        get { return wheel; }
        protected set { wheel = value; }
    }
}

class FastCar : Car
{
    public new FastCarWheel Wheel
    {
        get { return base.Wheel as FastCarWheel; }
        set { base.Wheel = value; }
    }
}

class SlowCar : Car
{
    public new SlowCarWheel Wheel
    {
        get { return base.Wheel as SlowCarWheel ; }
        set { base.Wheel = value; }
    }
}

You might want to evaluate if your base class is doing too much. It might be possible to solve your problem by splitting your classes in to many smaller classes. On the other hand, sometimes it's unavoidable.

Michael Meadows
+1  A: 

I would create an ICar and then define your Cars that way, instead of an abstract class

interface ICar
{
   IWheel Wheel {get; set;}
}

class FastCar: ICar
{
   FastWheel fastWheel;
   IWheel Wheel
   {
      get { return fastWheel; }
      set
      {
          if (value is FastWheel) fastWheel = (FastWheel)value;
      }    
   }         
}

class SlowCar: ICar
{
   SlowWheel slowWheel;
   IWheel Wheel
   {
      get { return slowWheel; }
      set
      {
          if (value is SlowWheel ) slowWheel = (SlowWheel )value;
      }    
   } 
}

class FastWheel: IWheel {}
class SlowWheel: IWheel {}
Joseph
+1  A: 

Since your goal appears to be allowing the client code to get the property back as a WheelPart, but only set it as a specific subclass you have a couple of options. Though I'm afraid that neither of them are very clean.

Firstly you could throw a runtime error if the wrong type is set:

    public abstract class Car
    {
        public abstract WheelPart Wheel { get; set; }
    }

    public class FastCar : Car
    {
        private FastWheel _wheel;
        public override WheelPart Wheel
        {
            get { return _wheel; }
            set
            {
                if (!(value is FastWheel))
                {
                    throw new ArgumentException("Supplied wheel must be Fast");
                }
                _wheel = (FastWheel)value;
            }
        }
    }

But I wouldn't do this as it is very unclear to the client code that any other type of wheel will throw an exception, and they'll get no compiler feedback.

Otherwise you could separate out the Getter and Setter for the property so that the type required is very clear:

    public abstract class Car
    {
        public abstract WheelPart Wheel { get; }
    }

    public class FastCar : Car
    {
        private FastWheel _wheel;
        public override WheelPart Wheel
        {
            get { return _wheel; }
        }

        public void SetWheel(FastWheel wheel)
        {
            _wheel = wheel;
        }
    }

This is much clearer to the client and IMHO a nicer solution if you absolutely must expose the getter as the base WheelPart class.

Martin Harris
The second solution seems pretty nice.
Greg