5

Right now I'm working with some code that combines state and operations.

It looks something like this (note: doesn't actually deal with Cars/Trucks. I'm abstracting the business logic here, and apologize in advance that the analogy doesn't entirely make sense)

TruckImpl {
    // Fields/Properties
    int Mileage
    bool MissionAccomplished
    // ... and many, many more

    // Constructor
    TruckImpl TruckImpl(IRentalAgreement agreement)

    // methods
    void Drive() {
        // Sets a whole bunch of fields/properties which are subsequently used to make decisions in Park()
        GetCarFromDealer()
        Haul()
    }
    void GetCarFromDealer()
    void Haul()

    // Return to rental company if done, otherwise park at home
    void Park()
}


CarImpl {
    // Fields
    int Mileage
    bool MissionAccomplished

    // Constructor
    CarImpl CarImpl(IRentalAgreement agreement)

    // methods
    // Sets a whole bunch of fields/properties which are subsequently used to make decisions in Park()
    void Drive() {
        // Sets a whole bunch of fields/properties which are subsequently used to make decisions in Park()
        GetCarFromDealer()
        Race()
    }
    void GetCarFromDealer()
    void Race()

    // Return to rental company if done, otherwise park at home    
    void Park()
}

It's used in some code that looks similar to the following pseudocode:

RentalVehicleHandler {
    IVehicleImplFactory factory;

    Handle(IRentalAgreement agreement) {
        impl = factory.Create(agreement)

        impl.Drive()
        impl.Park()
    }
}

Main() {
    while(true) {
    var agreements = GetAgreements(); 

    foreach(var agreement in agreements) {
        handler.Handle(agreement)
    }

}

In the above code, we're getting batches of "RentalAgreements", but still processing each of them individually. Now, I want to handle Cars and Trucks slightly separately. For Trucks, I will still Drive() then Park() each individually. For Cars, I want to GetCarFromDealer() individually, but then Race() the entire batch of cars at once before Parking them. I might want to do something similar for Trucks in the future, but maybe not.

I thought that it would be helpful to separate my state from my implementation like below: (to start, no batching-related stuff added yet):

// One per rental agreement
RentalVehicle {
    #Properties
    RentalAgreement agreement
    int Mileage
    bool MissionAccomplished
    //(...all of the TruckImpl propertiies?)
}

// Can handle any number of RentalAgreements, maybe multiple
CarDriver {

    RentalVehicle Drive(RentalVehicle vehicle)
    RentalVehicle Park(RentalVehicle vehicle)
}

TruckDriver {
    RentalVehicle Drive(RentalVehicle vehicle)
    RentalVehicle Park(RentalVehicle vehicle)
}

Is this a helpful/reasonable thing to do? Or is it just a bunch of extra work that will gain me no advantage? I think it will help me both with the problem at hand and help me with further refactoring, but I've also tried a few other things that were dead-ends and I don't want to spend a bunch of time refactoring this for no reason.

1

This is perfectly reasonable. In fact, that change is a textbook example of the functional programming paradigm, which is gradually becoming more popular these days.

The key ideas from functional programming that are relevant here are that you should strive to have mostly pure functions with no state or side effects and mostly immutable variables that get defined once and never change. When striving for this, it's typical to end up writing code like State finalState = changeStuff(initialState), which is basically what you have in your last snippet. If you do this, I would go all the way and make RentalVehicle an immutable type (which I think in C# is done by making its properties readonly).

The benefit of pure immutable things is simply that they are a lot easier to reason about than mutable stateful side-effecty things. If a function has no side effects, and its return value depends solely on what arguments you pass it, then it's not going to surprise you very often. The main drawback (if we ignore performance concerns) is that the real world is stateful, and so are many of the things that our code has to do. Going 100% functional for all of your renting, hauling and racing logic is probably not a good idea unless you have enough functional programming experience to find such code intuitive (or you're working in a functional language that has tools like monads to make it easier). However, splitting your code into "pure" and "impure" parts is often a good idea no matter what you're working on, as it limits the places where you have to worry about error-prone mutable state manipulations (again, assuming any potential performance loss is a non-issue).

I can't tell you whether that particular part of the codebase would actually benefit from more purity, but it's certainly not an unreasonable thing to try.

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.