Matthieu Vergne's Homepage

Last update: 14/10/2019 21:30:41

Resolving God Objects:
State Dispatch

Context

This article is part of the God object resolution series. To illustrate it, we use a running example through the whole series. So far, we dispatched the logics of the God object into new features, while keeping the state centralized in the God object. This article is about the dispatch of this state.

Problem

Each feature depends on the state of the God object instead of having its own. We want instead to have complete features to build on, in other words to extract the state into the relevant features.

Solution

In our running example, we have reached this state:


	class EmployeeManager {
		// Initial state
		private int nextEmployeeId = 0;
		private final Map<Integer, Employee> employees = new HashMap<>();
		private final Map<Integer, Address> addresses = new HashMap<>();
		private final Map<Integer, String> phoneNumbers = new HashMap<>();
		private final Map<Integer, Double> salaries = new HashMap<>();
		
		// New features building on it
		private final Employer employer = new EmployerImpl(() -> ++nextEmployeeId, () -> employees);
		private final EmployeeDetails details = new EmployeeDetailsImpl(() -> addresses, () -> phoneNumbers, () -> employees);
		private final SalarySettings salarySettings = new SalarySettingsImpl(() -> salaries);
		
		...
	}

At this point, we can identify which parts of the state should go to which features. The most trivial case is the salaries map, which is only used by the salariesSettings feature. For this kind of trivial case, we can simply move the map of the God class into the new feature. As a reminder, the current SalarySettingsImpl class is the following:


	class SalarySettingsImpl implements SalarySettings {
		private final Supplier<Map<Integer, Double>> salariesMap;

		public SalarySettingsImpl(Supplier<Map<Integer, Double>> salariesMap) {
			this.salariesMap = salariesMap;
		}

		/** Change the salary of the given {@link Employee}. */
		public void setSalary(int employeeID, double salary) {
			salariesMap.get().put(employeeID, salary);
		}

		/** Return the salary of the given {@link Employee}. */
		public double getSalary(int employeeID) {
			return salariesMap.get().get(employeeID);
		}

		/** Raise the salary of the given {@link Employee}. */
		public void raiseSalary(int employeeID, double amount) {
			Map<Integer, Double> salaries = salariesMap.get();
			salaries.put(employeeID, salaries.get(employeeID) + amount);
		}
	}

We now replace the salariesMap supplier by the actual salaries map:


	class SalarySettingsImpl implements SalarySettings {
		private final Map<Integer, Double> salaries = new HashMap<>();
		
		// No parametered constructor needed anymore, we only use the default constructor.

		/** Change the salary of the given {@link Employee}. */
		public void setSalary(int employeeID, double salary) {
			salaries.put(employeeID, salary);
		}

		/** Return the salary of the given {@link Employee}. */
		public double getSalary(int employeeID) {
			return salaries.get(employeeID);
		}

		/** Raise the salary of the given {@link Employee}. */
		public void raiseSalary(int employeeID, double amount) {
			salaries.put(employeeID, salaries.get(employeeID) + amount);
		}
	}

This modification allows us to simplify the God object:


	class EmployeeManager {
		// Initial state
		private int nextEmployeeId = 0;
		private final Map<Integer, Employee> employees = new HashMap<>();
		private final Map<Integer, Address> addresses = new HashMap<>();
		private final Map<Integer, String> phoneNumbers = new HashMap<>();
		// No salaries map anymore
		
		// New features building on it
		private final Employer employer = new EmployerImpl(() -> ++nextEmployeeId, () -> employees);
		private final EmployeeDetails details = new EmployeeDetailsImpl(() -> addresses, () -> phoneNumbers, () -> employees);
		private final SalarySettings salarySettings = new SalarySettingsImpl(/*no param anymore*/);
		
		...
	}

Now, the SalarySettingsImpl class is complete: it integrates both the logics and its related state. When we created this class, we didn't spend much time on its naming because our objective was to move all the existing code to implement the feature. Now that we have all of it, we can think better about the specifics of this class (relatively to its interface) to rename it. In our case, we could rename it MemoryBasedSalarySettings because it stores the information in local memory. We could rename it MapBasedSalarySettings to be more specific regarding the kind of memory storage (we could use other structures in other implementations). We could use yet again other naming conventions based on what seems the most relevant for the situation at hand. Although the naming of the class is out of scope regarding the God object issue, this is the right time to think about it.

After moving the salaries map into its feature, we can look for the other fields of the God class. addresses and phoneNumbers are also used by a single feature: EmployeeDetailsImpl. Like salaries, we can move them directly into the feature:


	class EmployeeManager {
		// Initial state
		private int nextEmployeeId = 0;
		private final Map<Integer, Employee> employees = new HashMap<>();
		// No addresses map anymore
		// No phoneNumbers map anymore
		
		// New features building on it
		private final Employer employer = new EmployerImpl(() -> ++nextEmployeeId, () -> employees);
		private final EmployeeDetails details = new EmployeeDetailsImpl(/*two params removed*/ () -> employees);
		private final SalarySettings salarySettings = new SalarySettingsImpl();
		
		...
	}

	class EmployeeDetailsImpl implements EmployeeDetails {
		private final Map<Integer, Address> addresses = new HashMap<>();
		private final Map<Integer, String> phoneNumbers = new HashMap<>();
		private final Supplier<Map<Integer, Employee>> contractsMap;

		public EmployeeDetailsImpl(Supplier<Map<Integer, Employee>> contractsMap) {
			// No supplier for addresses anymore
			// No supplier for phone numbers anymore
			this.contractsMap = contractsMap;
		}

		/** Return the {@link Address} of the given {@link Employee}. */
		public Address getAddress(int employeeID) {
			return addresses.get(employeeID);
		}

		/** Change the {@link Address} of the given {@link Employee}. */
		public void setAddress(int employeeID, Address address) {
			addresses.put(employeeID, address);
		}

		/** Return the phone number of the given {@link Employee}. */
		public String getPhoneNumber(int employeeID) {
			return phoneNumbers.get(employeeID);
		}

		/** Change the phone number of the given {@link Employee}. */
		public void setPhoneNumber(int employeeID, String phoneNumber) {
			phoneNumbers.put(employeeID, phoneNumber);
		}

		/**
		 * Replace the {@link Employee}'s contractual details and returns the previous
		 * version.
		 */
		public Employee setContract(int employeeID, Employee employee) {
			return contractsMap.get().put(employeeID, employee);
		}

		/** Return the {@link Employee}'s contractual details. */
		public Employee getContract(int employeeID) {
			return contractsMap.get().get(employeeID);
		}
	}

Notice we remain with the contractMap supplier. In the God class, this parameter is filled with the emploees field. This field is shared between several features: EmployerImpl and EmployeeDetailsImpl. Consequently, we cannot simply move it in the features, because we would break the integrity of the data. You may face a case where the shared object is itself a God object, in which case you should refactor it like we refactor EmployeeManager, but it can be postponed until EmployeeManager is done. If we use it as a genuine shared state, features depending on it should receive it as an external dependency, through their constructor or a setter. EmployeeDetailsImpl can thus be refactored like this:


	class EmployeeManager {
		// Initial state
		private int nextEmployeeId = 0;
		private final Map employees = new HashMap<>();
		
		// New features building on it
		private final Employer employer = new EmployerImpl(() -> ++nextEmployeeId, () -> employees);
		private final EmployeeDetails details = new EmployeeDetailsImpl(employees);
		private final SalarySettings salarySettings = new SalarySettingsImpl();
		
		...
	}

	class EmployeeDetailsImpl implements EmployeeDetails {
		private final Map<Integer, Address> addresses = new HashMap<>();
		private final Map<Integer, String> phoneNumbers = new HashMap<>();
		private final Map<Integer, Employee> contracts;

		public EmployeeDetailsImpl(Map<Integer, Employee> contracts) {
			this.contracts = contracts;
		}

		...

		/**
		 * Replace the {@link Employee}'s contractual details and returns the previous
		 * version.
		 */
		public Employee setContract(int employeeID, Employee employee) {
			return contracts.put(employeeID, employee);
		}

		/** Return the {@link Employee}'s contractual details. */
		public Employee getContract(int employeeID) {
			return contracts.get(employeeID);
		}
	}

It means that the state of EmployeeDetailsImpl is partially shared: while it contains the addresses and phone numbers, the details about the contracts are stored elsewhere. If such a design is not wanted, it can be refactored later based on your own requirements, but such a refactoring goes out of our scope. This observation also applies to EmployerImpl, which also builds on it and can be refactored the same way:


	class EmployeeManager {
		// Initial state
		private int nextEmployeeId = 0;
		private final Map<Integer, Employee> employees = new HashMap<>();
		
		// New features building on it
		private final Employer employer = new EmployerImpl(() -> ++nextEmployeeId, employees);
		private final EmployeeDetails details = new EmployeeDetailsImpl(employees);
		private final SalarySettings salarySettings = new SalarySettingsImpl();
		
		...
	}

	class EmployerImpl implements Employer {
		private final Supplier<Integer> newEmployeeID;
		private final Map<Integer, Employee> employees;

		public EmployerImpl(Supplier<Integer> newEmployeeID, Map<Integer, Employee> employees) {
			this.newEmployeeID = newEmployeeID;
			this.employees = employees;
		}

		/** Add a new {@link Employee} and returns its ID. */
		public int hireEmployee(Employee employee) {
			int employeeId = newEmployeeID.get();
			employees.put(employeeId, employee);
			return employeeId;
		}

		/** Fire an {@link Employee} and returns its ID. */
		public Employee fireEmployee(int employeeID) {
			return employees.remove(employeeID);
		}
	}

Notice that, although we named it contracts in EmployeeDetailsImpl, we still call it employees here. This is because the expected usage is not the same, and because EmployeeDetailsImpl may require further refactoring to deal with Contract instances. The decisions taken for EmployeeDetailsImpl remain independent of the oens taken for EmployerImpl, which has its own purpose.

The last piece of the God object state is nextEmployeeId, which is only required by EmployerImpl. Like the previous trivial cases, it means that we can move it to the feature. Notice that it is not just a field, but also an associated processing (incrementation). We should move both of them inside the feature:


	class EmployeeManager {
		// Initial state
		// No nextEmployeeID anymore
		private final Map<Integer, Employee> employees = new HashMap<>();
		
		// New features building on it
		private final Employer employer = new EmployerImpl(/*one parameter less*/ employees);
		private final EmployeeDetails details = new EmployeeDetailsImpl(employees);
		private final SalarySettings salarySettings = new SalarySettingsImpl();

		...
	}

	class EmployerImpl implements Employer {
		private int nextEmployeeId = 0;
		private final Map<Integer, Employee> employees;

		public EmployerImpl(Map<Integer, Employee> employees) {
			// No employeeID supplier anymore
			this.employees = employees;
		}

		/** Add a new {@link Employee} and returns its ID. */
		public int hireEmployee(Employee employee) {
			int employeeId = ++nextEmployeeId;
			employees.put(employeeId, employee);
			return employeeId;
		}

		/** Fire an {@link Employee} and returns its ID. */
		public Employee fireEmployee(int employeeID) {
			return employees.remove(employeeID);
		}
	}

With this last refactoring, we have now dispatched all the state of the God object. Only the state shared between several features should remain at this point, since it cannot be dispatched to a single feature. The God class now looks like a fa├žade but is still bound to all its dependencies, since it implements them and stores their shared state. But before to dig in that matter, we should take a detour to the tests. Indeed, the tests of the God object are actually the tests of the features, so we must dipatch these tests before to consider the features as done.