7

I'm trying to practise following SOLID principles.

I'm a bit puzzled about following example (which is a remodel/artificial example based on actual code, which i cannot post here):

public class Driver{
    ICar car;
    public Driver(ICar car){
        this.car = car;
    }

    public void Drive(){
        car.Drive(this);
    }

    public void ChangeCar(ICar car){
        this.car = car;
    }
}
public class ICar {
    void Drive(Driver owner);
}
public class ExampleCar : ICar{
    private int fuel;

    public ExampleCar(int fuel){
        this.fuel = fuel;
    }

    public void Drive(Driver owner){
        if(fuel > 0){
            fuel--;
            Console.Writeline("driving exampleCar. Fuel: " + fuel);
            return;
        }
        owner.ChangeCar(new AnotherCar(200));
    }
}
public class Main{
    ICar car = new ExampleCar(100);
    Driver mcLaren = new Driver(car);

    for(int i = 0; i < 200; i++){
        mcLaren.Drive();
    }
}

So what is essentially going on here is "drive ExampleCar, if ExampleCar fuel is empty switch to AnotherCar"

A sort of "pseudo Finite State Machine" (not sure if this is even close to a finite state machine, but this is the best intepretation that i am able to understand).

So with this I am trying to achieve a change of state based on a condition, which only the current state is capable of checking (which is if fuel is empty).

It doesn't seem logical to shift the condition to the driver class and have it checking that condition, as it would violate the open close principle (assuming i want to put a new ICar, which has a completely different condition or no condition at all for instance).

However, this current implementation seems to violate the Dependency Inversion principle, as ICar is dependent on Driver.

I am not sure what to do here, or whether or not this implementation is ok.

I would appreciate your thoughts on this.

  • "but this current implementation seems to violate the Dependency Inversion principle, as ICar is dependend on Driver" - what hinders you from introducing an additional interface IDriver and make ICar only depend on that? – Doc Brown Sep 17 at 11:12
  • My lack of immagination i guess :D But on a serious note, would that "fix" everything? So you suggest creating an interface, make Driver implement it, and that interface contains the changecar method? sounds reasonable. i guess i shouldve thought about this sooner – Alan Sep 17 at 11:22
  • 5
    I would say your example is way too artificial and lacks way too much context to make a statement if it is "ok" or not. SOLID, and especially the DIP are not an end in themselves, they are means to an end, and your code does not automatically become "better" by introducing an interface for every class. – Doc Brown Sep 17 at 11:42
  • 2
    But let me add one point: something which looks to me like a "code smell" is where ExampleCar.Drive has a (potentially unexpected) side effect to the driver object. Changing the reference to a completely new car object inside of "Drive" looks unobvious to me - "refueling" the existing car object would probably more obvious. This might cause confusion and could be a violation of the principle of least astonishment. So there are more useful programming principles than just "SOLID". – Doc Brown Sep 17 at 11:47
  • 1
    @BenCottrell good point, i was cought up too much in DiP and all that, that i forgot the most basic principle. ty – Alan Sep 17 at 14:47
9

It's not appropriate to have the Car deciding what the owner should do when it's out of gas, and certainly not acceptable for the car to choose what other kind of car the driver should drive if it runs out of gas.

The ICar interface should look more like this:

public class ICar {
    // return false if the car is undriveable
    boolean Drive();
}

Then:

public class Driver{
    ICar car;
    public Driver(ICar car){
        this.car = car;
    }

    // false if the car is undriveable
    public boolean Drive(){
        return car.Drive();
    }

    public void ChangeCar(ICar car){
        this.car = car;
    }
}

public class Main{
    ICar car = new ExampleCar(100);
    Driver mcLaren = new Driver(car);

    for(int i = 0; i < 200; i++){
        while (!mcLaren.Drive()) {
            car = new AnotherCar(200);
            mcLaren.changeCar(car);
        }
    }
}

This is just the Single Responsibility Principle at work -- You decided at the outset that it's Main's job to decide what car the driver drives, that it's the Driver's job to drive the car he's given, and that it's the ICar's job to be driven as long as that's possible.

In order to do its job, Main needs to know that sometimes cars become undriveable.

3

For artificial examples out of any context, it is often undecidable if they violate SOLID or not, and it is impossible to evaluate the code as "good" or "bad" in a sensible manner . For example, the DIP says

"High-level modules should not depend on low-level modules. Both should depend on abstractions (e.g. interfaces)."

but what in this example is "a high level module" and what is a "low level module"? That is probably a matter of taste in such a contrived Car/Driver example. One can see both the Car and the Driver classes together as just "one module", or one can see each of them as a module of its own. For the latter case, it is quite simple to solve the mentioned DIP issue: just introduce another IDriver interface and make ICar depend on that instead of Driver.

But is this "better"? Adding the interface makes the code a little bit more complicated, and if that fulfills no purpose, it is a violation of the KISS principle.

Another detail in your code is that strange-looking side effect in ExampleCar.Drive. Changing the reference to a completely new car object inside of Drive looks unobvious to me - "refueling" the existing car object would probably be more obvious, like

 public void Drive(Driver owner){
    if(fuel > 0){
        fuel--;
        Console.Writeline("driving exampleCar. Fuel: " + fuel);
        return;
    }
    fuel=200;
    Console.Writeline("exampleCar refuled. Fuel: " + fuel);
 }

So maybe this could be a violation of the "principle of least astonishment". If it is really a violation can only be evaluated in context of a real application, with real classes and real use cases. Maybe there are reasons why the original side-effect is necessary, and maybe in the "real" context, the effect does not look that much unexpected to a user of those classes.

In short, there are more programming principles than just SOLID, applying them often requires trade-offs, and artificial examples usually lack enough context to evaluate them as "good" or "bad" design.

1

Most of the interaction between a car and driver shouldn't take place using public methods of either. Instead, a driver wishing to board a car should create a private object that is designed to receive notifications from a car, and pass that to a function which will "board" the car. The car should then give the driver a reference to a private "car control" object that the driver can then use to operate it.

If wrapper objects are used in this fashion, and they are expressly invalidated when a driver leaves a car, that will ensure that a car can't erroneously receive commands from a driver who's no longer in it, and a driver can't erroneously receive status updates from a car that someone else is driving.

1

I'm a bit puzzled about following example:

Are you not the creator of the example? The posted code, together with your question indicates that you are worrying about principles far before you have gotten your problem down, analyzed it and created a fair design to solve it, no matter how small or large it is.

You say:

A sort of "pseudo Finite State Machine" (not sure if this is even close to a finite state machine, but this is the best intepretation that i am able to understand).

Technically, every computer program is a finite-state machine. Oops, wrong... Let me rephrase:

Technically, every computer program is a finite-state machine. Notice how the focus is on the word "technically"? This is an intentional device for me to highlight what appears to be wrong with your viewpoint.

You are using Object-Oriented programming, classes, interfaces, encapsulation and stuff, you worry about Object-Oriented design principles and try to follow them, but you are not thinking objects, you think technicalities.

So, let me tell you the unconventional truth: Technically, your code is perfect! It (probably) does what you want it to, it is correct, functional and error-free, it may even be performant! That is, technically.

The problem is, Object-Oriented Design/Programming is not a methodology to follow blindly to solve every problem, rather a philosophy to assist you in analyzing a problem which you can, then, solve technically. If your problem stems from the real-world (note that it might just as well NOT stem from the real world), Object-Oriented Design fits hand-in-glove for you. You can think of the natural objects, relationships between them, how natural boundaries exist that cannot be violated and how each object is responsible for itself. Objects may be composed of other objects, upon which they depend.

Let's try to apply some object-like thinking to your case/problem (which I am not even sure what it is, but anyway), borrowing from the corresponding real-world concepts you are trying to model. While at it, forget SOLID for a while.

  1. Cars do not contain Drivers and Drivers do not contain Cars. They can coexist in separate moments in time without depending on each other, that is, a Car can exist without a Driver, and a Driver without a Car. In simple words, you see empty Cars and Drivers walking on the street all the time.

That is to say, any representation of a Car or a Driver will NOT depend on a Driver or a Car (accordingly), or, in other words, implementations of type Driver or child thereof should not have any Car in their constructor. Accordingly, implementations of Car or specializations thereof should not have any Driver in their constructor.

  1. Drivers and Cars can get together, i.e. meet, to perform what we call "Driving". Drivers drive Cars, i.e. Cars are driven by Drivers and this action cannot occur when Driver and Car are separated from each other. A Driver may drive any Car and a Car may be driven by any Driver. Also, a Driver does not drive a Car for his entire life, only for a given distance/time each time.

When two classes meet each other, a reference of an instance of one of them may end up inside the code of an instance of the other one. When they have to be together, constantly, it usually is time for constructor injection, so they can be "married". When they have to be together occasionally, think about method injection, where they meet, do what they do together, and then forget each other. Remember that we are modelling the act of driving here, hence the logic. If a Car were to be used as a sleeping place, for example, our analysis would be quite different.

Point 2 means that we expect a method somewhere in either a Car or a Driver (or both), taking a parameter of the other. Basing on the verb and the transitivity in the analysis of point 2 (a Driver drives a Car), it seems that... well, the Driver drives the Car. A Car having a method named "Drive" taking a Driver parameter does not make sense in the real-world, because a car does not drive a Driver. Also, a Driver must explicitly state how far (or how long) they will drive the Car.

public interface IDriver
{
    void Drive(ICar car, double miles);
}
  1. A Car cannot go forever, it needs refueling every once in a while. The fuel tank is inside the car, no functional Car exists without a fuel tank and no Car functions without fuel. But a Car consumes fuel based on driven distance (yes, I know, and a dozen other factors too; I'm just being overly simplistic for a while). Therefore, a Car cannot just be "driven", it needs to be driven for a distance. Finally, Cars consume different amounts of fuel based on the distance and, also, Drivers can check up on the fuel at will, and they can refuel them.

Putting this into perspective, a Car spends fuel, but only the Car itself knows how. That is a secret of the Car. Drivers being able to check up on the fuel means that the remaining fuel must not be hidden inside the car. Here, the lines begin to blur a bit. Should the fuel be public or private? It depends on your use cases. If you are going to support a whole lot of things based on the exact amounts of fuel, sooner or later, you may have to make it public. Let's omit the case for the moment, you only care if a car can move or not.

Point 3 makes it clear that a Car spends fuel in its own manner, so a Driver just drives and the fuel diminishes. Because object-oriented programming allows you enough flexibility, you can ask a car to move, then let it tell you whether it moved and how much it moved. If a car may fail to move for a variety of reasons, you can create the technical counterpart of some... reasons!

public interface ICar
{
    bool Move(double miles, out double actuallyTravelledMiles);
}

Not using your imagination, but thinking in real-world objects, you can even do:

public enum CarTripResult
{
    Successful,
    OutOfGas,
    NoIgnition,
    //...
}

public interface ICar
{
    CarTripResult MakeTrip(double miles, out double travelledMiles);
}

So, before you know it, based on the above code, you have:

public class Car : ICar
{
    private double fuelCapacity_in_gallons;

    private double fuel_in_gallons;

    private double consumptionInMPG;

    public Car(double mpgConsumption, fuelCapacity)
    {
        consumptionInMPG = mpgConsumption;
        fuelCapacity_in_gallons = fuelCapacity;
    }

    //Simplistic representation of the action of Refueling.
    public void Refuel(double gallons)
    {
        fuel_in_gallons += gallons;
        if (fuel_in_gallons > fuelCapacity_in_gallons)
        {
            fuel_in_gallons = fuelCapacity_in_gallons;
        }
    }

    public CarTripResult MakeTrip(double miles, out double travelledMiles)
    {
        double neededFuel = miles / consumptionInMPG; //to get gallons.

        if (fuel_in_gallons > neededFuel)
        {
            fuel_in_gallons -= neededFuel;
            travelledMiles = miles;
            return CarTripResult.Successful;
        }
        else
        {
            //Calculate how much you can travel in miles.
            double distanceCapacity = fuel_in_gallons * consumptionInMPG;

            travelledMiles = distanceCapacity;
            return CarTripResult.OutOfGas;
        }
    }
}

public class Driver : IDriver
{

    public void Drive(ICar car, double miles)
    {
        CarTripResult = car.MakeTrip(miles, out double actualMiles);

        Console.WriteLine("Travelled a distance of " + actualMiles.ToString("0.00"));

        //The code from this point on will depend on
        //what it is you are trying to achieve.
        if (CarTripResult != CarTripResult.Successful)
        {
            //Do something, depending on your actual problem/scenario.
        }
        else if (...)
        {
            //...etc
        }
    }
}

public class Application
{
    public void Main()
    {
        //Remember, parameters are (mpg, capacity).
        ICar cheapCar = new Car(20, 40);
        ICar expensiveCar = new Car(10, 40);

        cheapCar.Refuel(40);
        expensiveCar.Refuel(40);

        Driver driver = new Driver();

        driver.Drive(cheapCar, 50);
        driver.Drive(expensiveCar, 50);
    }
}

Now that the domain logic has been analyzed and followed relatively closely, how "SOLID"-ly does this resulting design seem to be evolving? Notice that we didn't even once think about SOLID principles while laying down the logic. Also, one thing to remember here is that this entire design is based on a specific need to tackle a specific problem. If your perspective of the exact same domain were different, your solution, design and corresponding code might end up much different.

To (finally) answer your original question(s), you do not treat the SOLID principles as the primary guide to a design. Your guide should be the domain you are trying to model, combined with the problems you are trying to solve. If you follow the domain closely and carefully, things will fall into place on their own. This may sound like an oversimplification, but in any case, it is easy for the SOLID principles to get in the way of your attempts to unfold your model, especially at the beginning, while your design is incomplete. Do not focus on the SOLID principles at the expense of making a proper "object-oriented", domain-driven (well, if applicable...) analysis of your problem. Record the objects, study the relationships, represent them, make a few prototypes. The real-world is often rather "SOLID", so let this "SOLID"-ity translate to your domain.

In short, focus on your design and "invoke" the principles after you have something. The principles come after you start designing, not to show you if you are doing things right (remember, "right" is a technicality), but how much trouble you are going to have in the long run.

  • 1
    really REALLY appreciate the detailed explanation. If i understood you correctly, you are saying that i should base my code on the actual concept of Car and Driver while ignoring SOLID at first, in order to have those objects have the proper relationships (in my case, driver beeing injected into car using the constructor should be replaced by a type of method injection because drivers arent always inside their car objects). So does this essentially mean "code/model first, SOLID later" ? – Alan Sep 18 at 7:37
  • 1
    It's a "creative" combination of two co-evolving processes, of which you should not neglect either one. Think of it as "oscillating" back-and-forth between coding and checking the quality of your assumptions. It's like writing an essay for a 2-hour exam and the examiner will check and correct it afterwards. You cannot "develop" your thoughts as effectively if the examiner is all the time above your head, "correcting" your every sentence. They most likely cannot know enough about what you are trying to convey, at least not until after your text begins to acquire a specific "shape"... – Vector Zita Sep 18 at 8:42
  • ...so, while not being above your head all the time helps, it might also help if they do come above your head after every other paragraph, when your text has certainly begun making enough sense to judge and correct. So, to "solidify" the metaphor(!), "SOLID" rules check the quality of the "shape" of your code, but first, you have to have a shape to check. If you try to evaluate your code "applying SOLID" when it's too early, you will get more "false positives", which would simply self-resolve after spending a bit more time thinking, re-thinking and polishing your domain representation. – Vector Zita Sep 18 at 8:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.