Casting good practice

Question

Hi,

I've got 3 classes

namespace ServerPart
{

    public class Car
    {
    }

    public class SUV:Car
    {
        public string Name {get;set;}    
        public string Color {get;set;)
    }
}

And

namespace WebSericePart
{
    public class Car
    {
    }
}

namespace WebSericePart.Car
{
    public class SUV
    {
        public string Name {get;set;}

        public string Color {get;set;)
    }
}

And I've translator

namespace WebServicepart.Translators
{
    public static class ModelToContract
    {
        public Car[] ToCars(ServerPart.Car[] modelCars)
        {
            List<Car> contractCars=new List<Car>();
            foreach(ServerPart.Car modelCar in modelCars)
            {
                contractCars.Add(ToCar(modelCar);
            }
            return contractCars.ToArray();
        }

        public Car ToCar(ServerPart.Car modelCar)
        {
            if(modelCar is ServerPart.SUV)
            {
                return ToSUV(modelCar);
            }
            else
            {
                throw new NotImplementedException("Not supported type of Car");
            }
        }

        public Car ToSUV(ServerPart.Car modelCar)
        {
            SUV suv = new SUV();

            suv.Name=((ServerPart.SUV)modelCar).Name;
            suv.Color=((ServerPart.SUV)modelCar).Color;

            // ?? Is good practice ?? Or 
            //ServerPart.SUV  suv=(ServerPart.SUV)modelCar
            //suv.Name=suv.Name
            //suv.Color=suv.Color
            // is better ??

            return suv;
        }
    }
}

Do I used some else bad practices ?? Or Everything is OK :) ?

Answer 1

There's nothing wrong with using a property inline with a cast ((Type)object).Property.

I prefer the second method because you're reducing duplicate code. If you add some properties later, you will not have to keep duplicating the cast code.

Answer 2

Avoid cast, use a visitor pattern if you can modify ServerPart.

Answer 3

I actually prefer the second options. I think it is most readable and better yet, easier to debug. Perhaps you are lacking a null check, but maybe it is not neccesary as it can never happen.

ANYWAY, both are OK, I think.

Answer 4

This isn't a casting problem, but in general, returning an array isn't such a good idea in C#:

public Car[] ToCars(ServerPart.Car[] modelCars)

See Eric Lippert's excellent article Array's Considered Somewhat Harmful for the reason why.

This is particularly true since internally, the Car objects are stored in a list. If the method calling ToCars only needs to get a sequence of Cars, you could use IEnumerable<Cars> (see below). Other interfaces can grant the caller other accesses as well - see ICollection<T> and IList<T>.

public IEnumerable<Car> ToCars(IEnumerable<ServerPart.Car> modelCars)
{
  List<Car> contractCars=new List<Car>(); 
  foreach(ServerPart.Car modelCar in modelCars) 
  { 
    contractCars.Add(ToCar(modelCar); 
  } 
  return (IEnumerable<Car>)contractCars;
}