Avoiding two-way dependency when correlating derived classes from different base classes

Question

I'm working on a model that do some stuff with a bunch of different Vehicles. Every Vehicle is supposed to do stuff, but each Vehicle type does a different stuff. So I implemented it this way, using .NET Framework:

abstract class Vehicle
{
   abstract void DoStuff()
}

class Car : Vehicle
{
   override void DoStuff()
   {
       //Do some Car stuff here
   }
}

class Motorcycle : Vehicle
{
   override void DoStuff()
   {
       //Do some Motorcycle stuff here
   }
}
class Model
{
  RunModel(Vehicle[] vehicleCollection)
  {
    foreach(Vehicle currentVehicle in vehicleCollection)
    {
      currentVehicle.DoStuff()
    }
  }
}

This is the core funcionallity of my program and it's working as expected. Now I'm supposed to output reports based on the stuff each Vehicle has done. Each type of Vehicle is supposed to ouput a different kind of Report, so I made a similar solution for it:

abstract class Vehicle
{
   abstract void DoStuff();
   abstract Report GetReport();
}

class Car : Vehicle
{
   override Report GetReport()
   {
       return new CarReport(this);
   }
}

class Motorcycle : Vehicle
{
   override Report GetReport()
   {
       return new MotorcycleReport(this);
   }
}

abstract class Report
{
   int Foo {get; set;}

   Report (Vehicle _vehicle)
   {
       Foo = _vehicle.CommonProperty;
   }
      
}

class CarReport : Report
{
   string Bar {get; set;}
   CarReport(Car _car) : base(_car)
   {
       Bar = _car.CarPropoerty;
   }
}

class MotorcycleReport : Report
{
   bool Baz {get; set;}
   MotorcycleReport(Motorcycle _cycle) : base(_cycle)
   {
       Baz= _cycle.MotorcyclePropoerty;
   }
}
class Model
{
  RunModel(Vehicle[] vehicleCollection)
  {
    foreach(Vehicle currentVehicle in vehicleCollection)
    {
      currentVehicle.DoStuff()
      currentVehicle.GetReport()
    }
  }
}

This is working fine too, but the problem is that Car and Motorcycle now depends on CarReport and MotorcycleReport. Since this is non-core functionallity to my program and the Report structure may change a lot in future versions, I'd like to implement it in a way that the Reports depends on the Vehicles, but the Vehicles do not depend on the Reports.

I've tried an external overloaded method that gets a Vehicle and outputs the proper Report Or passing an abstract Report (or interface IReport) to the Vehicle "GetReport" method But since my RunModel method doesn't know what type of Vehicle it is dealing with, I can't find a way to map it to the right Report type.

Is there a way to avoid this two-way dependency?

Answer 1

You're right about keeping the core domain as simple as possible. It should only have to deal with its own complexity, with as little interference and dependencies from the outside.

First thing that comes to mind is that even though inheritance may make sense for the Vehicle hierarchy. Question is, does it make sense for the reports? Will you ever use the abstract base Report class by itself? The one with only the common properties.

If it does

You can use a manager to take over the responsibility of creating Reports.

public class ReportManager
{
    public Report GetReport<T>(T vehicle) where T : Vehicle
    {
        switch (vehicle)
        {
            case Car car:
                return new CarReport(car);

            case Motorcycle motorcycle:
                return new MotorcycleReport(motorcycle);

            default:
                throw new NotImplementedException(vehicle.ToString());
        }
    }
}

You can use it like this.

public class Model
{
    private readonly ReportManager _reportManager;

    public Model(ReportManager reportManager)
    {
        _reportManager = reportManager;
    }

    public List<Report> RunModel(Vehicle[] vehicles)
    {
        var reports = new List<Report>();

        foreach (var vehicle in vehicles)
        {
            vehicle.DoStuff();
            reports.Add(_reportManager.GetReport(vehicle));
        }

        return reports;
    }
}

If it doesn't

You can divide the work over two separate flows.

public class Model
{
    public List<CarReport> CarReports { get; private set; }
    public List<MotorcycleReport> MotorcycleReports { get; private set; }

    public void RunModel(Vehicle[] vehicles)
    {
        // 1. Do stuff
        foreach (var vehicle in vehicles)
        {
            vehicle.DoStuff();
        }
        // 2. Get reports
        CarReports = vehicles.OfType<Car>().Select(car => new CarReport(car)).ToList();
        MotorcycleReports = vehicles.OfType<Motorcycle>().Select(motorcycle => new MotorcycleReport(motorcycle)).ToList();
    }
}

The difference

The first method returns a list of base class. The second method stores lists of different types on the object. Once you have different types, you can no longer return them in a typed collection without upcasting first.

Final thoughts

Report structure may change a lot in future versions

You could implement an enum ReportType on Vehicle. Imagine a future request to create different reports for muscle cars and family cars. Instead of diving deeper into inheritance, you could then generate different reports based solely upon the enum value.

Answer 2

Dependency injection might help. Built in dependency injection in .Net Core doesn’t give the option to switch between two different implementations of IReport but you can inject an implementation of ICarReport into Class Car and an implementation of IMotorcycleReport into Class Motorcycle. Then, you can swap out the implementations if they change without changing the Classes that depend on them.

There are other IoC containers, like Lightinject, that do allow you to inject different implementations of IReport called named dependencies. You may want to search for something like that.

Also, I’m not sure if you’re using .Net Core or .Net Framework. .Net Core has built in dependency injection but you’ll need to install a Nuget package like Lightinject or Ninject for the .Net Framework.

Edit:

It sounds like you are looking for a design pattern to achieve Inversion of Control (IoC). In that case, as the different answers point out, you can use the Factory Pattern, Service Locator Pattern or Dependency Injection Pattern.

Dependency Injection may not work for you if your project is old or already very large. In that case it may be something to look into for your next project. The Factory Pattern might be exactly what you're looking for in this case. It all depends on a lot of details we don't know at this point.

Also, there will be varying opinions on the different patterns but there are often many patterns that can be used to solve a particular design problem.

Answer 3

You could use generics to create a link between a report and its corresponding vehicle. Based on this information, you can create report based on vehicle.

The vehicles and reports are like this:

    public abstract class Vehicle
    {

    }
    public class Car : Vehicle
    {

    }
    public class Motorcycle : Vehicle
    {

    }

    public abstract class Report
    {

    }

    public abstract class Report<T> : Report where T:Vehicle
    {
        int Foo { get; set; }

        public Report(T _vehicle)
        {

        }
    }
    public class CarReport : Report<Car>
    {
        string Bar { get; set; }
        public CarReport(Car _car) : base(_car)
        {

        }
    }
    public class MotorcycleReport : Report<Motorcycle>
    {
        bool Baz { get; set; }
        public MotorcycleReport(Motorcycle _cycle) : base(_cycle)
        {

        }
    }

1 - We could use reflection to generate the Report object depending on the current Vehicle:

public abstract class Vehicle
    {
        public Report GetReport()
        {
            var genericReportType = typeof(Report<>).MakeGenericType(this.GetType());
            var reportType = Assembly.GetExecutingAssembly().GetTypes().Where(x => genericReportType.IsAssignableFrom(x)).Single();
            return Activator.CreateInstance(reportType, this) as Report;
        }
    }

If we need to optimize performance, we could cache a dictionary:

public abstract class Vehicle
    {
        private static Dictionary<Type, Type> vehicleToReport;

        static Vehicle()
        {
            var reports = Assembly.GetExecutingAssembly().GetTypes().Where(x => typeof(Report).IsAssignableFrom(x) && x.IsAbstract == false);

            vehicleToReport = reports.ToDictionary(x => x.BaseType.GetGenericArguments().Single(), x => x);
        }
        public Report GetReport()
        {
            var reportType = vehicleToReport[this.GetType()];
            return Activator.CreateInstance(reportType, this) as Report;
        }
    }

I'd like to implement it in a way that the Reports depends on the Vehicles, but the Vehicles do not depend on the Reports.

2 - If you want to remove the Report dependency completely from Vehicle. You can create a factory class and move the GetReport method from your Vehicle class to this factory method.

You can implement the factory method (we usually call it factory design pattern). There are 2 options to implement this factory method:

a) Use reflection like above to dynamically discover new reports for new vehicles thanks to the generic implementation of the reports and vehicles.

b) Just hardcode the mappings vehicleToReport below to map Vehicle to Report:

public class ReportFactory
    {
        private static Dictionary<Type, Type> vehicleToReport;

        static ReportFactory()
        {
            //Build the mappings dynamically using reflection or just hardcode it.
            var reports = Assembly.GetExecutingAssembly().GetTypes().Where(x => typeof(Report).IsAssignableFrom(x) && x.IsAbstract == false);

            vehicleToReport = reports.ToDictionary(x => x.BaseType.GetGenericArguments().Single(), x => x);
        }
        public Report GetReport(Vehicle vehicle)
        {
            var reportType = vehicleToReport[vehicle.GetType()];
            return Activator.CreateInstance(reportType, vehicle) as Report;
        }
    }

Answer 4

You can remove your Dependency using Factory class. Here is a sample code.

class Program
{

    static void Main(string[] args)
    {
        // Set your Report Builder Factory (Concrete / Dynamic)
        ConcretReportBuilderFactory concreteReportBuilderFactory = new ConcretReportBuilderFactory();

        DynamicReportBuilderFactory dynamicReportBuilderFactory = new DynamicReportBuilderFactory();

        Vehicle[] vehicleCollection = new Vehicle[]
        {
            new Car(concreteReportBuilderFactory),
            new Motorcycle(dynamicReportBuilderFactory)
        };

        RunModel(vehicleCollection);

        Console.ReadKey();
    }

    static void RunModel(Vehicle[] vehicleCollection)
    {
        foreach (Vehicle currentVehicle in vehicleCollection)
        {
            currentVehicle.DoStuff();
            var vehicleReport = currentVehicle.GetReport();
        }
    }
}

public abstract class Vehicle
{
    protected readonly ReportBuilderFactory reportBuilderFactory;
    // I'm using Constructor Injection, but you can use Property or Method injection 
    // if you want to free your constructor remaining parameter less.
    public Vehicle(ReportBuilderFactory reportBuilderFactory)
    {
        this.reportBuilderFactory = reportBuilderFactory;
    }

    public abstract void DoStuff();
    public abstract Report GetReport();
    public string CommonProperty { get; set; }
}

public class Car : Vehicle
{
    public Car(ReportBuilderFactory reportBuilderFactory) : base(reportBuilderFactory)
    {
    }
    public override void DoStuff()
    {
        //Do some Car stuff here
    }

    public override Report GetReport()
    {
        return this.reportBuilderFactory.GetReport(this);
    }
}

public class Motorcycle : Vehicle
{
    public Motorcycle(ReportBuilderFactory reportBuilderFactory) : base(reportBuilderFactory)
    {
    }
    public override void DoStuff()
    {
        //Do some Motorcycle stuff here
    }
    public override Report GetReport()
    {
        var report = this.reportBuilderFactory.GetReport(this);
        return report;
    }
}

public abstract class Report
{
    public Report(Vehicle vehicle)
    {
        Foo = vehicle.CommonProperty;
    }
    public string Foo { get; set; }

    public virtual void ShowReport()
    {
        Console.WriteLine("This is Base Report");
    }
}

[ReportFor("Car")] // (Pass class name as argument) .For the implementation of DynamicReportBuilderFactory.
public class CarReport : Report
{
    string Bar { get; set; }
    public CarReport(Car _car) : base(_car)
    {
        Bar = _car.CommonProperty;
    }
    public override void ShowReport()
    {
        Console.WriteLine("This is Car Report.");
    }
}

[ReportFor("Motorcycle")] // (Pass class name as argument) .For the implementation of DynamicReportBuilderFactory
public class MotorcycleReport : Report
{
    public MotorcycleReport(Vehicle vehicle) : base(vehicle)
    {
    }

    public override void ShowReport()
    {
        Console.WriteLine("This is Motor Cycle Report.");
    }
}    

[AttributeUsage(AttributeTargets.Class)]
public class ReportFor : Attribute
{
    public string ReportSource { get; private set; }
    public ReportFor(string ReportSource)
    {
        this.ReportSource = ReportSource;
    }
}

public abstract class ReportBuilderFactory
{
    public abstract Report GetReport(Vehicle vehicle);
}

// Static Implementation . this is tightly coupled with Sub Classes of Report class.
public sealed class ConcretReportBuilderFactory : ReportBuilderFactory
{
    public override Report GetReport(Vehicle vehicle)
    {
        switch (vehicle)
        {
            case Car car:
                return new CarReport(car);

            case Motorcycle motorcycle:
                return new MotorcycleReport(motorcycle);

            default:
                throw new NotImplementedException(vehicle.ToString());
        }
    }
}

// Dynamic Implementation . this is loosely coupled with Sub Classes of Report class.
public sealed class DynamicReportBuilderFactory : ReportBuilderFactory
{
    private readonly Dictionary<string, Type> _availableReports;

    public DynamicReportBuilderFactory()
    {
        _availableReports = GetAvailableReportTypes();
    }

    static Dictionary<string, Type> GetAvailableReportTypes()
    {
        var reports = Assembly.GetExecutingAssembly()
                         .GetTypes().Where(t => typeof(Report).IsAssignableFrom(t)
                                             && t.IsAbstract == false
                                             && t.IsInterface == false
                                             // Geting classes which have "ReportFor" attribute
                                             && t.GetCustomAttribute<ReportFor>() != null 
                                         );

        // You can raise warning or log Report derived classes which dosn't have "ReportFor" attribute
       // We can split ReportSource property contains "," and register same type for it . Like "CarReport, BikeReport"

        return reports.ToDictionary(x => x.GetCustomAttribute<ReportFor>()?.ReportSource);
    }

    public override Report GetReport(Vehicle vehicle)
    {
        var reportType = _availableReports[vehicle.GetType().Name];
        return Activator.CreateInstance(reportType,vehicle) as Report;
    }
}

Answer 5

To avoid the two-way dependency, Vehicles should depend on abstractions. As well as Reports. As the wise man said: "High level modules should not depend upon low level modules. Both should depend upon abstractions." This is my suggestion, it was made trying to mess as little as possible with your sample program. The Model.RunModel method still doesn't know what type of Vehicle it is dealing with, and that's OK. The method GenerateReport in CarReportGenerator (and MotorcycleReportGenerator) is the right place to do concrete actions on concrete reports. Future changes in Report structure should be addressed there.

Let's cut to the code:

 abstract class Vehicle
    {
        public int CommonProperty { get; set; }

        public abstract void DoStuff();

        public abstract Report GetReport();

    }

    class Car : Vehicle
    {
        public string CarProperty { get; set; }

        public override void DoStuff()
        {
            //Do some Car stuff here
            Console.WriteLine("Doing Car stuff here.");
        }

        public override Report GetReport()
        {
            // Injecting dependency
            CarReportGenerator crpt = new CarReportGenerator(this);
            
            return crpt.GenerateReport();
        }
    }

    class Motorcycle : Vehicle
    {
        public bool MotorcycleProperty { get; set; }

        public override void DoStuff()
        {
            //Do some Motorcycle stuff here
            Console.WriteLine("Doing Motorcycle stuff here.");
        }

        public override Report GetReport()
        {
            // Injecting dependency
            MotorcycleReportGenerator mrpt = new MotorcycleReportGenerator(this);
            return mrpt.GenerateReport();
        }
    }

    abstract class Report
    {
        public int Foo { get; set; }
    }

    class CarReport : Report
    {
        public string Bar { get; set; }
    }

    class MotorcycleReport : Report
    {
        public bool Baz { get; set; }
    }

    class Model
    {
        internal void RunModel(Vehicle[] vehicleCollection)
        {
            foreach (Vehicle currentVehicle in vehicleCollection)
            {
                currentVehicle.DoStuff();
                //currentVehicle.GetReport();
                Report rpt = currentVehicle.GetReport();
                Console.WriteLine(rpt.Foo);
            }
        }
    }

    interface IReportGenerator
    {
        Report GenerateReport();
    }

    class CarReportGenerator : IReportGenerator
    {
        private Car _car;
        
        public CarReportGenerator(Vehicle car)
        {
            _car = (Car)car;
        }

        public Report GenerateReport()
        {
            CarReport crpt = new CarReport();

            // acces to commom Property from Vehicle
            crpt.Foo = _car.CommonProperty;
            // acces to concrete Car Property from Car
            crpt.Bar = _car.CarProperty;
            // go on with report, print, email, whatever needed

            return crpt;
        }
    }

    class MotorcycleReportGenerator : IReportGenerator
    {
        private Motorcycle _motorc;

        public MotorcycleReportGenerator(Vehicle motorc)
        {
            _motorc = (Motorcycle)motorc;
        }

        public Report GenerateReport()
        {
            MotorcycleReport mrpt = new MotorcycleReport();

            // acces to commom Property from Vehicle
            mrpt.Foo = _motorc.CommonProperty;
            // acces to concrete Motorcycle Property from Motorcycle
            mrpt.Baz = _motorc.MotorcycleProperty;
            // go on with report, print, email, whatever needed

            return mrpt;
        }
    }

The dependency between Vehicles and Reports is gone. If a new kind of Vehicle and Report were to be added later, it shold be done without changes on what is working now.

// A whole new vehicle, report and report generator.

    class Quad : Vehicle
    {
        public double QuadProperty { get; set; }

        public override void DoStuff()
        {
            //Do some Quad stuff here
            Console.WriteLine("Doing Quad stuff here.");
        }

        public override Report GetReport()
        {
            // Injecting dependency
            QuadReportGenerator crpt = new QuadReportGenerator(this);

            return crpt.GenerateReport();
        }
    }

    class QuadReport : Report
    {
        public double Doe { get; set; }
    }
    class QuadReportGenerator : IReportGenerator
    {
        private Quad _quad;

        public QuadReportGenerator(Vehicle quad)
        {
            _quad = (Quad)quad;
        }

        public Report GenerateReport()
        {
            QuadReport crpt = new QuadReport();

            // acces to commom Property from Vehicle
            crpt.Foo = _quad.CommonProperty;
            // acces to concrete Quad Property from Quad
            crpt.Doe = _quad.QuadProperty;
            // go on with report, print, email, whatever needed

            return crpt;
        }
    }

However, a new dependency exists between Vehicles and ReportGenerators. It could be addressed by creating an interface for vehicles IVehicle and another for reports IReport, so Vehicles and ReportGenerators depend on it. You may go another step beyond and create a new interface IVehicleReport, containing the method Report GetReport(). This way present and future concerns of vehicles and reports are kept apart.