Abstract factory design for different cars

Is there anything wrong with this?

```
namespace WindowsFormsApplication1
{
public interface ICarfactory
{
ICar getCar(CarType carType);
}

public interface ICar
{
}

public class Sedan: ICar
{
}

public class Hatchback : ICar
{
}

public class SUV : ICar
{
}

public class HondaSUV : SUV
{
}

public class HondaSedan : Sedan
{
}

public class HondaHatchback : Hatchback
{
}

public class HyundaiSUV : SUV
{
}

public class HyundaiSedan : Sedan
{
}
public class HyundaiHatchback : Hatchback
{
}

public class MarutiHatchback : Hatchback
{
}

public class MarutiSUV : SUV
{
}

public class MarutiSedan : Sedan
{
}

public class Hyundai : ICarfactory
{
public ICar getCar(CarType carType)
{
switch (carType)
{
case CarType.Sedan:
return new HyundaiSedan();
case CarType.HatchBack:
return new HyundaiHatchback();
case CarType.SUV:
return new HyundaiSUV();
default:
return null;
}
}

}

public class Maruti : ICarfactory
{
public ICar getCar(CarType carType)
{
switch (carType)
{
case CarType.Sedan:
return new MarutiSedan();
case CarType.HatchBack:
return new MarutiHatchback();
case CarType.SUV:
return new MarutiSUV();
default:
return null;
}
}
}

public class Honda : ICarfactory
{
public ICar getCar(CarType carType)
{
switch (carType)
{
case CarType.Sedan:
return new HondaSedan();
case CarT

The factory in itself is nice (except for one thing), but I have one big problem with your code.

Approach:

You are heavily relying on inheritance over composition. Instead of something so complicated as running that against a Factory, where you return types after a switching an enum you should IMO do the following:

public abstract class Car 
{
    private Manufacturer _manufacturer;
    public Manufacturer Manufacturer 
    {
        get 
        { 
            return _manufacturer; 
        }
        set 
        {
            _manufacturer = value; 
        }
    }

    private CarType _carType;
    public CarType CarType
    {
        get
        {
            return _carType;
        }
        //You can even make that stuff read-only by removing the setter
        set
        {
             _carType = value;
        }
     }
}

Implementation:

When you pass an invalid CarType to the Factory you silently return null. That's somewhat unfriendly to the user. Factories are expected to either return a meaningful value or throw an exception. If you allow returning of null, you make the user of the factory do additional null-checks. That is not nice...

Instead you should do the following:

default:
    throw new IllegalArgumentException("CarType not found");

StackExchange Code Review Q#52070, answer score: 5