Friday, November 14, 2008

IF-LESS code : State Pattern

Hi all, let's start with a post on this interesting topic (at least for me) with a simple example.
We have an interface named IService that represents a generic service. The interface offers three methods : start(), stop() and isRunning(). 

public interface IService {
void start(); // Starts the service.
void stop(); // Stops the service. 
boolean isRunning(); // Returns true if the service is running.
}

While looking at the following implementation, I wasn't satisfied about the conditional statements on start() and stop() method :

public class ServiceImpl implements IService {
   private volatile boolean started;
   private Thread thread;
   public synchronized void start() { 
      if (started) {
         return;
      }
      
      started = true;
      
      thread = ... // Create your daemon implementation.
      thread.setDaemon(true);
      thread.start();
   }
   
   public synchronized void stop()
   {      
      if (!started) {
         return;
      }
      started = false;
      thread.interrupt();
      
      try {
         thread.join();
      }
      catch (InterruptedException ignore) {         
      }
   }
   
   public synchronized boolean isRunning() {
      return started;
   }
}

To be honest, what I don't like is :

- Maintaining the state of the service in a boolean member and therefore relying on that in order to know if the service is alive or not (the isRunning method)
- Conditional logic used for checking everytime the (start & stop) methods are called if the service is alive or not. I think the service should know that without using a boolean member to remember! An example : if you are driving a car, how do you know that it's running? You know that because it's running and you're driving! ;-) and not because after turned it on you put a post-it on the dashboard with "RUNNING" written over! 
I mean : it's the status itself that suggests you what's going on!
As result of that, I prefer this version of the service implementation:


public class IfLessService implements IService {
   private Thread _watcher;
   
   /**
    * RUNNING STATE.
    */
   private final IService running = new IService() {
      public boolean isRunning()
      {
         return true; // We are inside the running state and so...
      }
      public void start() {
         // Nothing to do here...it is already started.
      }
      public void stop()  {
         _watcher.interrupt();      
         try  {
            _watcher.join();
         }
         catch (InterruptedException ignore)
         {         
         }
        state = notRunning;
      }
   };
   
   /**
    * NOT RUNNING STATE.
    */
   private final IService notRunning = new IService()  {      
      public boolean isRunning()  {
         return false; // We are inside the not running state and so...
      }
      public synchronized void start()  {
      _watcher = ...// Create your daemon implementation...
         _watcher.setDaemon(true);
         _watcher.start();
         // ...and make a state change too...from NOT-RUNNING to RUNNING.
         state = running;
      }
      public void stop() {
         // Nothing to do here...it is already stopped.         
      }      
   };   
   
   // Default initial state is stopped (not running).
   private IService state = notRunning;
   public synchronized void start() {      
      state.start();  // Current state delegation
   }
   public synchronized void stop() {      
      state.stop();    // Current state delegation   
   }
   public boolean isRunning() {
      return state.isRunning();  // Current state delegation
   }
}
Now the service implementation is delegating the execution of the IService methods to the current internal state. Those states are themselves implementors of IService interface. We have two states : running and notRunning. 
Note that the responsibility of  each state is not only to manage the "state" of the object at a specific moment but also to provide an eventual state transition: for example when the service is not running (state = notRunning), if you call the start() method there will be a state transition (state = running).
Any comment would be very very appreciated...

3 comments:

Federico Zancan said...

Hi Andrea,

I think your solution is brilliant. Btw, there's just a question that matters to me (I do not know Java, JVM and JIT compiler internals, so please forgive me for asking it): which added value comes up with this approach in terms of performance?
I mean, no matter your solution is pretty smart, but what I can see from my point of view is a whirlwind of code around the fact that I could have been faster if I accessed directly a boolean member within the class. Finally, you still need a member variable, anyways.
I'm not sure to understand your design reasons (and sure it's my fault), even if I think the result is very coherent, but - to be shorter - what about performance? Please light me up. Fede

Anonymous said...

Hi,
very useful to avoid 'if' and to put evidence on the state, either. The only thing I've a doubt about is the state knowledge; it would be probably better if a state didn't know the other state to set when leaving itself.
But it's probably a matter of choosing the level of complexity, as that would mean having a complete state machine...

Bye,
m.

Andrea said...

Hi, thanks for your post.
Yes, generally speaking I have the same doubt :) but the inner classes are so cool... :)
Maybe things could be better if we put the state changes in compose methods (something like turnOn() and turnOff() ) and let each state invoke those methods instead of working directly with state member instance.
Otherwise you could extract those states from the Context object but in this case you need a way to inject the current state (setCurrentState on Service)

Ciao,
Andrea