pfm: .plan, site map, about

Refactoring legacy code

I have recently refactored a piece of our product to improve its maintainability. It was a part responsible for (1) receiving messages from outside of the system, (2) calculating appropriate operation and optionally (3) adjusting the data in the database.

Unfortunately, the code was a total mess:

• a bunch of EJBs calling each other;
• working directly on the message data and on JPA entities;
• selecting and applying the appropriate operation was just a switch with a branch for each mode of operation.

public class UglyService {
	@EJB
	private EquallyUglyService otherService;

	public void processMessage(Message msg) {
		// ...
	}

	public boolean isOpen(Message msg) {
		return msg.getStatus() == Status.OPEN;
	}

	public void doSomething() {
		otherService.doSomething();
	}

	public void updateCase1(Entity ent, Message msg) {
		if (/* conditions */) {
			ent.setSomeProperty(msg.getPropertySource());
		}
	}

	public void updateCase2(Entity ent, Message msg) {
		if (/* other conditions */) {
			ent.setSomeOtherProperty(msg.getOtherPropertySource());
		}
	}
}

What I did was:

1. Create a strategy class for each case of that switch statement and only use original enumeration to request a specific strategy from a factory. Each of them implemented an interface.
2. Extract common parts of those strategies and introduce a composite that grouped strategies. (Factory was still hiding the fact that strategies were being composed as it always returned a reference to an implementation of an interface.)
3. Wrap the original message object in an adapter and move all service methods retrieving data from the message to the adapter. At this point the code started looking like object-oriented just a bit. At least it is more coherent now.
4. Implement a factory method in the above mentioned adapter to produce strategies based on its internal state.

public class SlightlyBetterService {
	public void processMessage(Message msg) {
		MessageAdapter ma = MessageAdapter.wrap(msg);
		Entity original = retrieveOriginal();

		// Adjuster is usually a composite of several different
		// strategies responsible for updates of different properties
		// of the object
		Adjuster adjuster = ma.produceAdjuster();
		adjuster.adjust(original);
	}
}

With the above enhancements, it is easy to introduce new actions, each of them is easy to test and if the objects (messages) received by our system change, only the adapter needs to be adjusted.

That was fun!

This work by Piotr Mieszkowski is licensed under CC-BY-SA 4.0