A time and place for everything

Last week at the pub, I got into a heated discussion about how to handle certain situations in retrospectives. It was a great discussion though I do wonder how other people would handle the situation. We got into the topic of dealing with safety during retrospectives, and what we can do to address safety. Someone mentioned a specific retrospective where, during the Art Gallery exercise, one person (out of a group of 25) had drawn a picture of many smiley faces and a single sad face. The Safety Check results didn’t indicate safety was a problem. The question posed was should the facilitator go ahead?

I gave a, rather rambling answer. Fair enough since it was a Friday, and well past normal working hours. Not to mention it was at the pub…

I said that, as a facilitator, you need to be constantly aware of what the situation at hand is. I explained that if I’m facilitating a meeting for someone else, particularly retrospectives, I’ll normally prepare by interviewing a few people first (informally, sometimes formally if need be). I like to know who’s going to be in the room, and what problems might surface, or what situation I might be stepping in to. I learned this tip from Kerths’ original book. I like to prepare the room before everyone enters, set up posters, whiteboards, flip charts, etc. It’s courteous to the participants, and I’m always hopeful they appreciate the effort that I made an effort, with the end result being better quality discussions. I’ll observe people as they enter, and watch the body language of people through the activities.

Given the particular example above, I can understand why the facilitator continued as they did. I often see the Art Gallery exercise used as an ice-breaker, a way of letting people have a bit of fun without being too serious. With a high safety check score, and the general mood of the group upbeat, I can understand why they proceeded.

Had many other circumstances been different, the mood of the group excessively sombre, or the safety check average versus significantly high, the outcome might be different. I talked about how, if one person out of that group had a real problem, I’d hope that there were other avenues to allow that person to express their discomfort. For example, one-to-one meetings is a great way of establishing this, or even tea and coffee breaks work with trusted co-workers does wonders. Retrospectives shouldn’t be the only place for dealing with issues.

Do you realise you’re facilitating?

A fundamental shift for people adopting agile practices is the move away from less directive management styles, to more facilitative management styles. This happens at all levels, be it the Project Manager, the Technical Lead, or simply the person arranging any meetings to order (for example, the Daily Stand Up, Retrospectives, Planning Games, etc). For those people in influential positions, I like to highlight their impact their facilitative style (good and/or bad) has on the team. Steven List is compiling a set of patterns for these, all very good and many of them which I’ve been witness to. Plenty of other books provide wonderful guides about this, like Jean Tabaka’s Collaboration Explained.

Controlling Time: Threaded Execution

Update (Feb 21): Thanks to some feedback from Taras and Lowell, the ThreadedRunner is the same as the java.util.concurrent.Executor class! I’ve updated the post to reflect this below. Thanks for the suggestion!

Unit testing threaded software can sometimes prove slightly cumbersome. Many of the examples on the net often show examples that tie a number of responsibilities together. It might be easy to see something like this:

  public class SomeExample {
    public void doSomeWork() {
      Runnable runnable = new Runnable() {
        public void run() {
          // do some long running work
        }
      };
      new Thread(runnable).start();
    }
  }

The thing about this class above is to realise what responsibilities it is mixing, including the new work they would like to (the item in Runnable), as well as how they would like to run it (the separate thread). It’s difficult to tease these two responsibilities apart the way it’s declared. The first thing I’d do is remove the dependency on the new thread. I’ll start by making use of the java.util.concurrent.Executor :

public interface Executor {
  void execute(Runnable runnable);
}

public class SingleThreadedExecutor implements Executor {
  public void execute(Runnable runnable) {
    runnable.run();
  }
}
public class NewThreadedExecutor implements Executor {
  public void execute(Runnable runnable) {
    new Thread(runnable).start();
  }
}

We’ll inject the SingleThreadedExecutor in our unit tests, so we only ever have one thread of execution, and we can rely on tests being consistently reproducible. We’ll inject the NewThreadedExecutor when we need to assemble our application for production. Here’s the step by step refactoring.

Wrap inside our new implementation

public class SomeExample {
  public void doSomeWork() {
    Runnable runnable = new Runnable() {
      public void run() {
        // do some long running work
      }
    };
    new NewThreadedExecutor().start(runnable);
  }
}

Inject instance via the constructor

public class SomeExample {
  private final NewThreadedExecutor executor;
  public SomeExample(NewThreadedExecutor executor) {
    this.executor = executor;
  }  

  public void doSomeWork() {
    Runnable runnable = new Runnable() {
      public void run() {
        // do some long running work
      }
    };
    executor.start(runnable);
  }
}

Replace specific with interface

public class SomeExample {
  private final Executor executor;
  public SomeExample(Executor executor) {
    this.executor = executor;
  }  

  public void doSomeWork() {
    Runnable runnable = new Runnable() {
      public void run() {
        // do some long running work
      }
    };
    executor.start(runnable);
  }
}

In the tests, you can now inject an instance of SingleThreadedExecutor, whilst in production mode, you use the NewThreadedExecutor.

Disclaimers
Of course, this still leaves your testing strategy incomplete where you do want to test your software with the number of threads that you do expect, particularly when you need to share state across different processes.

Retrospectives are not the only place for continual improvement

Teams adopting agile, and even frequently teams who consider themselves agile, often hit a stumbling block. Here’s how the thinking goes… agile is about improvement. Agile projects do retrospective to improve. Therefore, retrospectives = improvement and improvements (only) happen in retrospectives.

Unfortunately many teams suffer without realising improvements go beyond retrospectives. Everyday there is an opportunity to improve, an opportunity to learn. It sometimes takes a while to see them. It often takes much longer to unwind the restraints of organisational “process” on people’s desires to experiment and fail fast, and learn from those mistakes.

Don’t get me wrong. Retrospectives also have their place. Sometimes teams don’t have an environment safe enough and retrospectives are one way of helping establish some safety. It takes commitment from leaders to create this environment of safety, and something I encourage greatly when I work with teams.

Do you recognise your team falling in this pattern? Break out of them, and remind them that improvements don’t have to wait for a meeting to be attempted.

What differences cause your project death?

I’m not about to go and list all the thousand differences that will contribute to a slow and painful project death, but I will mention a few categories I always see.

Differences in property names
When you start to use build files and automated builds, you will end up with property files. Often each property will be named slightly differently, especially when used, and you need to constantly remember all the different things.

Differences in test styles
Some people will prefer to put test set up in the field of a class, others that aren’t.

Differences in path locations
In one module, you might have /src/java/com, in another you might have /executable/java/com.

Differences in configuration values
I remember one project where the team had over twenty different configuration values to allow them to put parts of the codebase wherever they liked. It was a nightmare when something would go wrong, trying to track down if the difference was environmental, or if it was a true mistake.

Differences in name
Ala ubiquitous language. The users know something by one name, the BAs call it by another, the developers by yet another one. Each time, compounding the already-difficult task of communication. Don’t you think meetings are long enough without having to continuously translate terms as well as make decisions?

Differences in code style
Everything included from curly brace placement, number of spaces to naming conventions of variables. Create a code standard for those things that really don’t make a significant difference so you can spend valuable thought cycles on other things.

Controlling Time: How to deal with infinity

It’s been a while since I’ve had to slice and dice legacy code. It reminds me how easily, non test driven code, slips into the abyss of tight coupling and the effort of retrofitting (unit) tests increasing in effort with time. Of course, I don’t believe TDD should be done all the time, but for most production code I do. In this entry, I’m going to use the Sprout Inner Class mechanism to demonstrate how to start making something that would have been untestable much more manageable.

The scenario
What we have below is the entry point into our program (i.e. a class with a main method). When I first encountered this class, it had too many responsibilities including, and not limited to, parsing command line options, some application logic, handling terminal signals, and wiring up all the objects for the program to start. Combined with an infinite working loop, you can imagine what a nightmare it was to unit test. Here’s the modified class, called RunningProgram representing the important parts of the class related to this walk through.

Our task: To reduce the number of responsibilities and test the body of the start method (we couldn’t simply extract method immediately due to the number of fields it modified)

package com.thekua.examples;

import sun.misc.Signal;
import sun.misc.SignalHandler;

public class RunningProgram implements SignalHandler {
	private boolean running = true;
	// some other fields

	public void start() {
		running = true;
		while (running) {
			// do some work (that we want to unit test)
			// it changes about ten fields depending on what condition
			// gets executed
		}
		// Finished doing some work
	}

	// it also had plenty of other methods

	public static void main(String [] args) {
		RunningProgram program = new RunningProgram();
		Signal.handle(new Signal("TERM"), program);
		Signal.handle(new Signal("INT"), program);
		program.start();
	}

	public void handle(Signal signal) {
	    // Program terminated by a signal
		running = false;
	}
}

Step 1: Understand the dependencies that make this test difficult to test
To get some level of functional testing around this program we had a number of timed triggers, watching for state modifications with a given timeout. Our problem was infinity itself, or rather, not sure when to interrupt infinity to watch for when the body of work inside the loop had finished its work once, twice, and not too early. We could have made it more complicated by introducing lifecycle listeners yet we hesitated at that option because we thought it would complicate the code too much.

We noticed the use of the running flag. We noticed it was the condition for whether or not we continued looping, and was also the trigger for a graceful shutdown using the sun.misc.Signal class. Notice that the running program implements the SignalHandler interface as a result. We thought that running behaviour of the RunningProgram could be extracted into a separate aspect.

Step 2: Encapsulate, encapsulate, encapsulate
Our first task, was to remove direct access to the running flag since the class modified it in two places. Sprout an inner class, and simply delegate to getters and setters and we might find we have a class that looks like:

package com.thekua.examples;

import sun.misc.Signal;
import sun.misc.SignalHandler;

public class RunningProgram implements SignalHandler {
    public static class RunningCondition {
	    private boolean running = true;

		boolean shouldContinue() {
		    return running;
		}

		public void stop() {
		    running = false;
		}
    }

	private RunningCondition condition = new RunningCondition();

	// some other fields

	public void start() {
		while (condition.shouldContinue()) {
			// do some work (that we want to unit test)
			// it changes about ten fields depending on what condition
			// gets executed
		}
		// Finished doing some work
	}

	// it also had plenty of other methods

	public static void main(String [] args) {
		RunningProgram program = new RunningProgram();
		Signal.handle(new Signal("TERM"), program);
		Signal.handle(new Signal("INT"), program);
		program.start();
	}

	public void handle(Signal signal) {
	    // Program terminated by a signal
		condition.stop();
	}
}

Moving the running flag to a separate class gives us a number of benefits. It lets us hide the implementation of how we handle running, and puts us down the road of clearly teasing apart the overloaded responsibilities.

Step 3: Consolidate related behaviour
It bothered me that the main program had the shutdown hook. That behaviour definitely felt strongly related to the RunningCondition. I felt it was a good thing to move it to the that class. We now have something that looks like:

package com.thekua.examples;

import sun.misc.Signal;
import sun.misc.SignalHandler;

public class RunningProgram {
    public static class RunningCondition implements SignalHandler {
	    private boolean running = true;

		boolean shouldContinue() {
		    return running;
		}

		public void handle(Signal signal) {
	        // Program terminated by a signal
			running = false;
	    }
    }

	private RunningCondition condition = new RunningCondition();

	// some other fields

	public void start() {
		while (condition.shouldContinue()) {
			// do some work (that we want to unit test)
			// it changes about ten fields depending on what condition
			// gets executed
		}
		// Finished doing some work
	}

	// it also had plenty of other methods

	public static void main(String [] args) {
		RunningProgram program = new RunningProgram();
		Signal.handle(new Signal("TERM"), program.condition);
		Signal.handle(new Signal("INT"), program.condition);
		program.start();
	}

}

Note that it is now the RunningCondition that now implements the SignalHandler interface (that we are using to register with the Signal

Step 3: Remove dependency chain
The difficulty with this class still exists. We cannot modify the RunningCondition of this program since it creates one for itself. Since I prefer Constructor Based Dependency Injection, I’m going to apply Introduce Parameter to Constructor, moving the field declaration to the constructor itself. Here it is what the class looks like now:

package com.thekua.examples;

import sun.misc.Signal;
import sun.misc.SignalHandler;

public class RunningProgram {
    public static class RunningCondition implements SignalHandler {
	    private boolean running = true;

		boolean shouldContinue() {
		    return running;
		}

		public void handle(Signal signal) {
	        // Program terminated by a signal
			running = false;
	    }
    }

	private final RunningCondition condition;

	public RunningProgram(RunningCondition condition) {
	    this.condition = condition;
	}

	// some other fields

	public void start() {
		while (condition.shouldContinue()) {
			// do some work (that we want to unit test)
			// it changes about ten fields depending on what condition
			// gets executed
		}
		// Finished doing some work
	}

	// it also had plenty of other methods

	public static void main(String [] args) {
		RunningProgram program = new RunningProgram(new RunningCondition());
		Signal.handle(new Signal("TERM"), program.condition);
		Signal.handle(new Signal("INT"), program.condition);
		program.start();
	}

}

Note that we are still bound to the implementation of the specific RunningCondition, so it’s time to apply Extract Interface, and understand what role that RunningCondition has. We chose the name, RunStrategy for the role name, and to help keep the names more aligned, we ended up renaming RunningCondition to RunLikeADaemonStrategy. Our code now looks like this:

package com.thekua.examples;

import sun.misc.Signal;
import sun.misc.SignalHandler;

public class RunningProgram {
    public static interface RunStrategy {
	    boolean shouldContinue();
	}

    public static class RunLikeADaemonStrategy implements SignalHandler, RunStrategy {
	    private boolean running = true;

		public boolean shouldContinue() {
		    return running;
		}

		public void handle(Signal signal) {
	        // Program terminated by a signal
		    running = false;
	    }
    }

	private final RunStrategy runStrategy;

	public RunningProgram(RunStrategy runStrategy) {
	    this.runStrategy = runStrategy;
	}

	// some other fields

	public void start() {
		while (runStrategy.shouldContinue()) {
			// do some work (that we want to unit test)
			// it changes about ten fields depending on what condition
			// gets executed
		}
		// Finished doing some work
	}

	// it also had plenty of other methods

	public static void main(String [] args) {
		RunLikeADaemonStrategy strategy = new RunLikeADaemonStrategy();
		RunningProgram program = new RunningProgram(strategy);
		Signal.handle(new Signal("TERM"), strategy);
		Signal.handle(new Signal("INT"), strategy);
		program.start();
	}
}

The best thing is that our RunningProgram no longer needs to know what happens if a terminate or interrupt signal is sent to the program. With simply a dependency on the RunStrategy we can know inject a fixed run strategy for tests that we ended up calling a RunNumberOfTimesStrategy. We also promoted the specific RunLikeADaemonStrategy to a full class (not an inner class).

Thanks for getting this far! Please leave a comment if you thought this was useful.