1

Background

I'm not sure my question in the title is actually expressed properly, but here's what I'm trying to do. (I am using a bogus analogy here to represent real code written by the team. We aren't allowed to post actual code. But I have tried to ensure that the concepts I see in our code base have been transferred over to this example)

I have some vehicle records in my database... some of them are cars, others will be trucks etc. I also have some controllers - endpoints for each vehicle type. So I have a Car controller that allows specific methods just for cars.

All of the columns in the database are identical for all types of vehicles. The vehicle DTO class looks like this:

 public class Vehicle
 {
        [Key]
        public int Id { get; set; }
        [Column]
        public int BrandName{ get; set; }
        [Column]
        public string VehicleDetails{ get; set; }

        public IDictionary<string, object> VehicleDetailsJson
        {
          get
          {
            return (IDictionary<string, object>)JsonConvert.DeserializeObject<List<ExpandoObject>>(VehicleDetails).FirstOrDefault();
          }
    }

But in the case of a car, we need additional fields. so we have this:

public class Car : Vehicle
{
  public static List<string> Models {get;} = new()
  {
     "two-seater",
     "four-seater",
     "six-seater"
  }

  public bool doSomethingToTheBrandName()
   {
      //does something to the brand string on the base class.
      return true; 
   }
}

Because all the database columns that are being return are the same, I'm using the vehicle model as a return type in my repository class. This is how I pull the data from the database: (pseudo-ish code)

    public class VehicleRepository: GenericRepository<Vehicle>, IVehicleRepository
     {
      private readonly string encryptionPassword;

     public VehicleRepository(AppSettings appSettings) : base(appSettings)
      {
        encryptionPassword = appSettings.EncryptionPassword;
      }

      public List<Vehicle> GetVehicles(string brandName= "")
      {
        var vehicles = new List<Vehicle>();

            var searchFilter = !string.IsNullOrEmpty(brand )
                               ? $"BrandName = @Brandname"
                               : "BrandName = 'all'";
            vehicles = Retrieve().ToList();
        
        return vehicles ;
    }

Here's the interface for the rpository:

public interface IVehicleRepository : IGenericRepository<Vehicle>
{
    List<Vehicle> GetVehicles(string brandName = "");
}

Problem

Where I'm running into issues is trying to call the repository from a car controller. This is where I'm not quite sure how to design / write the code.

The car controller uses requests and responses like this:

public class CarRequest: Request
{
    public string brandName{ get; set; }

}

public class CarResponse: Response
{
    public List<Car> Cars { get; set; }  //notice we are returning a car, not vehicle.
}

And here's the controller itself:

   public class CarController: ControllerBase
    {
        private readonly AppSettings appSettings;
        private readonly IVehicleRepository vehicleRepository;

    public CarResponse GetCars (CarRequest request)
    {
        try
        {
            var response = new GetCarResponse();
            response.Cars = vehicleRepository.Vehicles("ford");

        }
    }

Naturally, the system fails trying to save the response from the database call because I'm trying convert a vehicle into a car.

Questions

  • Which class should be responsible for casting from the base type "Vehicle" to "Car".
  • Also in the future, if there are Car specific methods that we need to write in a repository class, should we just do that inside the vehicle repo class?
  • Anything else that smells off?

EDIT 1

So my real code actually has an if statement baked in (I'm including the suggested refactors)

public List<T> GetVehicles<T>(string brandName= "", int someFlag)
  {
    var vehicles = new List<T>();
    if (someflag==0)
    {
         var searchFilter = !string.IsNullOrEmpty(brand )
                       ? $"BrandName = @Brandname"
                       : "BrandName = 'all'";
         vehicles = Retrieve().OfType<T>.ToList();
    }
    else
    {
       vehicles.Add(Retrieve(someFlag));
    }
    return vehicles ;
   }

I'm sorry but I'm still kinda new to OO and c#. So I wasn't sure if by TVehicle you meant T. I've seen before. But the error I'm getting on this line:

 vehicles = Retrieve().OfType<T>.ToList();

is

CS0119 'Enumerable.OfType(IEnumerable)' is a method, which is not valid in the given context

and I haven't attempted to fix the Else path but that also shows the following error:

CS1503 Argument 1: cannot convert from 'Widgets.Models.Vehicle' to 'T'

EDIT 2

   public List<TVehicle> GetVehicles<TVehicle>(string brandName= "", int someFlag)
where TVehicle: Vehicle
      {
        var vehicles = new List<TVehicle();
        if (someflag==0)
        {
             var searchFilter = !string.IsNullOrEmpty(brand )
                           ? $"BrandName = @Brandname"
                           : "BrandName = 'all'";
             vehicles = Retrieve().OfType<TVehicle>.ToList();
        }
        else
        {
           vehicles.Add(Retrieve(someFlag));
        }
        return vehicles ;
       }

But I'm still getting the same two errors on the same two lines.

dot
  • 14,928
  • 41
  • 110
  • 218
  • You can't "cast" from a base type to a derived type. The base type can't be guaranteed to have all of the information necessary to actually _be_ the derived type. See https://stackoverflow.com/questions/12565736/convert-base-class-to-derived-class – Logarr Jan 11 '23 at 21:22
  • 1
    `.OfType()`, because it's a method and method calls require the parentheses - also the name of the generic type parameter should be `T` when any type is allowed, but recommend using `TSomething` for more specific stuff. Like `TVehicle`, when you expect a vehicle type. – Mathieu Guindon Jan 11 '23 at 22:28
  • @MathieuGuindon doh! makes sense! that works. Any tips on the else{}? I'm getting the error cannot convert from Vehicle to TVehicle. – dot Jan 11 '23 at 22:31
  • `Retrieve()` should return `TVehicle` too, but then you'll hit another issue if you go and make it `Retrieve()` generic method (even with the same constraints).. you'll need to define `TVehicle` at the class level, i.e. you get a `public class VehicleService where... { ... }`, and then the methods can all refer to the same generic type parameter. The `.OfType<>()` call then ironically becomes redundant too! – Mathieu Guindon Jan 11 '23 at 22:37

2 Answers2

3

One of the solution to this is can be that, we will need to define the method generic so that we can pass the type from calling side something like:

  public List<TVehicle> GetVehicles<TVehicle>(string brandName= "")
  {
    var vehicles = new List<TVehicle>();

    var searchFilter = !string.IsNullOrEmpty(brand )
                       ? $"BrandName = @Brandname"
                       : "BrandName = 'all'";
    vehicles = Retrieve().OfType<TVehicle>.ToList();
    
    return vehicles ;
}

and now on calling side it should be something like:

var cars = GetVehicles<Car>("brand name");
Ehsan Sajjad
  • 61,834
  • 16
  • 105
  • 160
  • Sorry, I don't understand. What is TVehicle in this case? Still noob when it comes to oo and c# – dot Jan 11 '23 at 22:04
  • 1
    @dot it's a generic type. Surely you've seen types like `List` or `IEnumerable`? Methods can also be generic, so `TVehicle` is any type you want/need. You could constrain the type to be derived from a particular base class too, by adding a `where TVehicle : Vehicle` between the signature and the body of the method. – Mathieu Guindon Jan 11 '23 at 22:09
  • 1
    Sure, I'd go with 'C# generics', 'C# generic method', 'C# generic type parameters', and 'C# generic constraints'. – Mathieu Guindon Jan 11 '23 at 22:26
  • @MathieuGuindon cool! I mean, I tried the "where" clause. never knew I could do that. Unfortunately, I'm still getting some errors. Please see EDIT 2 which is where I tried to implement this answer. – dot Jan 11 '23 at 22:27
  • @dot what's the latest errors – Ehsan Sajjad Jan 12 '23 at 21:23
  • Edit 2 describes my latest state. – dot Jan 27 '23 at 13:44
1

Generally it is a good practice to have a separated repo for each domain entity. So it may worth creating an abstract Vehicle repository and derive other repose from it (not tested):

public abstract class VehicleRepository<TVehicle>: GenericRepository<TVehicle>, IVehicleRepository
where TVehicle:Vehicle
     {
      private readonly string encryptionPassword;

     public VehicleRepository(AppSettings appSettings) : base(appSettings)
      {
        encryptionPassword = appSettings.EncryptionPassword;
      }

      public List<TVehicle> GetVehicles(string brandName= "")
      {
        var vehicles = new List<TVehicle>();

            var searchFilter = !string.IsNullOrEmpty(brand )
                               ? $"BrandName = @Brandname"
                               : "BrandName = 'all'";
            vehicles = Retrieve().ToList();
        
        return vehicles ;
}

and then define TruckVehicle, CarVehicle ,.. inheriting from that class.

MD Zand
  • 2,366
  • 3
  • 14
  • 25