Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Basic question on refactoring into a abstract class

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

like image 537
Exist Avatar asked Apr 23 '09 18:04

Exist


4 Answers

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).

like image 93
dirkgently Avatar answered Oct 21 '22 14:10

dirkgently


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.

like image 20
Michael Meadows Avatar answered Oct 21 '22 12:10

Michael Meadows


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.

like image 1
Martin Harris Avatar answered Oct 21 '22 13:10

Martin Harris


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 {}
like image 1
Joseph Avatar answered Oct 21 '22 14:10

Joseph