Is there a clean way to model methods that only make sense depending on the current state of the object?

Question

Let's say that I have a class called Mission. This class is intended to represent a mission requested by a professor working in a faculty. Mission has a private enum field representing whether the mission is PLANNED, IN_PROGRESS, COMPLETED, CANCELED, ABORTED, etc.

There are some things that the specs require to be doable upon a Mission that only make sense depending on the current state of the mission. For instance, it would only make sense to requestReimbursement for a mission that is completed. On the other hand, it doesn't make sense to cancel a mission that is completed. Having all these methods inside the Mission class and throwing an exception whenever the wrong method is called doesn't seem like a clean approach.

An alternative solution could be to create a subclass for each value of the mission states. This can be done by making the class Mission abstract while holding a reference to a data class representing other information about the mission. Each subclass would have a method that represents the transition between the current state and and the one that follows. For instance:

public enum MissionState {PLANNED, IN_PROGRESS, ABORTED, COMPLETED, ...}
public abstract class Mission {
    private MissionDetails details;
    private MissionState state;
}

public class PlannedMission extends Mission {
   public PlannedMission(MissionDetails details) {
       super.details = details;
       super.state = MissionState.PLANNED;
  }

  public InProgressMission start() {
      return new InProgressMission(super.details);
  }
}

public class InProgressMission extends Mission {
   public InProgressMission(MissionDetails details) {
       super.details = details;
       super.state = MissionState.IN_PROGRESS;
  }

  public AbortedMission abort() {
      return new AbortedMission(super.details);
  }

  public CompletedMission complete() {
      return new CompletedMission(super.details);
  };
}

public class CompeltedMission extends Mission {
   public CompletedMission(MissionDetails details) {
       super.details = details;
       super.state = MissionState.COMPLETED;
  }

  public void requestReimbursement() {
      //
  }
}

While this approach manages to avoid the problem mentioned above, it's significantly more complex, and might not scale well if the specs were to change to include additional mission states.

The kind of system I'm trying to design seems very prevalent and I myself have interacted with multiple similar solutions as an end user, so I was wondering if there is a standard, clean approach to go about solving this problem.

Answer 1

What you've done here has a name. This is an internal Domain Specific Language (iDSL). It can be used like this:

void execute(PlannedMission plannedMission) {
    plannedMission
        .start()
        .complete()
        .requestReimbursement()
    ;
}

Or like this:

AbortedMission abort(PlannedMission plannedMission) {
    return plannedMission
        .start()
        .abort()
    ;
}

This lets you give names to the different paths (here execute & abort) through your states by listing the transitions in the path. You can even decouple this from the concrete implementation by making PlannedMission, and its linked types, extensible (abstract, an interface, whatever, just not final). That way these named paths don't need rewriting even if what they call out to does.

I've built iDSLs before. Mostly to solve construction problems. They are the overpowered version of Joshua Bloch's Builder Pattern which is really about simulating named arguments in languages that don't have them (like Java) without using setters so your object can be immutable. The using code looks the same but since it only uses one type you can call the methods in any order. The Bloch Builder is not a DSL.

You might look at this using code an be tempted to call it a Law of Demeter violation. If these were randomly collected types and methods from all over the code base I'd agree with you. No one promised you this crazy path would always work so don't whine when it breaks. However, these aren't random types. These were designed to work together. They will change together and be deployed together. These are all friends. Not friends of friends.

DSLs are very powerful and very specific. When you build one inside an existing language (why I called it internal) like Java or C# you get to leverage a lot of existing tooling for your little language.

But yes, setting it up is still a lot of work. Generally, don't reach for this unless it will be widely used in your code base to offset the extra work setting it up.

Right now you only have two paths through this. So there's a good reason this seems over engineered. It is. There's not a lot of need for the flexibility the DSL provides. There's not a lot of reason to break up these steps as different state transitions. But if you really can't stand allowing a abort call to happen when in the wrong state then this works.

However, consider the GoF's State Pattern. With this design you could always call abort. Even when it didn't make sense. And it wouldn't have to do type checking. Rather, when you send a abort message to a completed mission it would quietly do a whole lot of nothing.

This is in line with the Tell, Don't Ask philosophy. You don't have to know how everything turns out. You can make something else deal with that. This borrows from my favorite pattern. The null object pattern. Sometimes quietly doing nothing is the thing to do. You've likely already used it. The empty string ("") is a null object. We use it to tell things to print nothing all the time.

So you have a choice. Obsessively control the callable messages so they are always correct to send or just deal with whatever messages come correctly.

The nice thing about the second one is it allows your abstract mission object to be a lot more useful.

Answer 2

There is no perfect way to model runtime state transitions in a type-safe manner. Your suggestion is a valid approach, but it has the downside that code can still hold a reference to a previous state and call methods on it. E.g:

  var inProgressMission = plannedMission.start();
  var abortedMission = inProgressMission.abort();
  inProgressMission.complete(); // <- oops!

So it is not as type-safe as it looks. The state subclasses still need to dynamically check the underlying state and throw an exception if the current operation is invalid - which is what you wanted to avoid.

Fundamentally, a static type system cannot model state transitions happening at runtime. Some niche languages have attempted to solve this with stuff like linear types or monads, but it is not something that is practically useful in a Java-like language.

In general, I would recommend avoiding such state-transitions since they lead to temporal coupling. Usually it is better to have different states represented by distinct objects rather than having the same object transition through multiple states at runtime. In your code example there doesn't seem to be a necessary reason that a PlannedMission and a InProgressMission should be the same underlying object. Instead PlannedMission could be a factory that generates InProgressMission. There is no problem in having the same mission factory generate multiple missions.

Answer 3

Do the role object pattern, where each role is a mission state: https://martinfowler.com/apsupp/roles.pdf

Or, use visitor pattern, which “allows adding new virtual functions to a family of classes, without modifying the classes.”: https://en.m.wikipedia.org/wiki/Visitor_pattern#

Answer 4

in rust you can do this:

struct PLANNED;
struct INPROGRESS;
struct ABOARTED;
struct COMPLETED;
struct Mission<State> {
    detail: String,
    _marker: std::marker::PhantomData<State>,
}
impl<T> Mission<T> {
    fn new(detail: &str) -> Mission<PLANNED> {
        Mission {
            detail: detail.to_string(),
            _marker: std::marker::PhantomData,
        }
    }
}
impl Mission<PLANNED> {
    fn start(self) -> Mission<INPROGRESS> {
        Mission {
            detail: self.detail,
            _marker: std::marker::PhantomData,
        }
    }
}
impl Mission<INPROGRESS> {
    fn abort(self) -> Mission<ABOARTED> {
        Mission {
            detail: self.detail,
            _marker: std::marker::PhantomData,
        }
    }
fn completed(self) -&gt; Mission&lt;COMPLETED&gt; {
    Mission {
        detail: self.detail,
        _marker: std::marker::PhantomData,
    }
}

}
impl Mission<COMPLETED> {
    fn requestReimbursement(self) {
        // ...
    }
}

if you call requestReimbursement by Mission<PLANNED> or some other non-Mission<COMPLETED> type, which will can't be compiled.