Matthieu Vergne's Homepage

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

Resolving God Objects:
Running Example

Context

This article is part of the God object resolution series.

Problem

This resolution goes through several phases dispatched in several articles. Moreover, we deal with a complex object containing several features. To ease the understanding, proper illustrations and examples are required.

Solution

Let's go through the details of the running example we will use in the series. Here is the code of the God object:


	class EmployeeManager {
		private int nextEmployeeId = 0;
		private final HashMap<Integer, Employee> employees = new HashMap<>();
		private final HashMap<Integer, Address> addresses = new HashMap<>();
		private final HashMap<Integer, String> phoneNumbers = new HashMap<>();
		private final HashMap<Integer, Double> salaries = new HashMap<>();

		/** 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);
		}

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

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

		/** 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);
		}

		/** 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);
		}

		/** 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);
		}
	}

This implementation remains simple: the methods are mainly one-liners, with no parameter control, the constructor has no parameter, and the state of the object is already greatly decomposed. Yet, we can identify several features related to employees management: hiring and firing, personnal details (address and phone number), salary management. These are separate features because we can clearly identify separate contexts in which only some methods are required. More specifically, hiring and firing makes sense at the start and at the end of the employees contracts. Personnal details may be changed at any time during the contract, at the initiative of the employee. Salaries management may be changed at any time during the contract, at the initiative of the employer. For instance, it would be unnecessary to care about the dependencies required for hiring and salary management when the employee just need to change personnal data after a family move.

This God object is tested in this way:


	public class EmployeeManagerTest {

		@Test
		public void testEmployeeCanBeRetrievedAfterHiring() {
			Employee employee = new TestEmployee();
			EmployeeManager manager = new EmployeeManager();
			int employeeID = manager.hireEmployee(employee);
			assertEquals(employee, manager.getEmployee(employeeID));
		}

		@Test
		public void testEmployeesHaveDifferentIDs() {
			EmployeeManager manager = new EmployeeManager();
			int employeeID1 = manager.hireEmployee(new TestEmployee());
			int employeeID2 = manager.hireEmployee(new TestEmployee());
			int employeeID3 = manager.hireEmployee(new TestEmployee());
			assertNotEquals(employeeID1, employeeID2);
			assertNotEquals(employeeID1, employeeID3);
			assertNotEquals(employeeID2, employeeID3);
		}

		@Test
		public void testEmployeeCannotBeRetrievedAfterFiring() {
			Employee employee = new TestEmployee();
			EmployeeManager manager = new EmployeeManager();
			int employeeID = manager.hireEmployee(employee);
			manager.fireEmployee(employeeID);
			assertNull(manager.getEmployee(employeeID));
		}

		@Test
		public void testEmployeeDetailsCanBeChanged() {
			EmployeeManager manager = new EmployeeManager();
			Employee employee = new TestEmployee();
			int employeeID = manager.hireEmployee(employee);
			Employee newDetails = new TestEmployee();
			manager.setEmployee(employeeID, newDetails);
			assertNotEquals(employee, manager.getEmployee(employeeID));
			assertEquals(newDetails, manager.getEmployee(employeeID));
		}

		@Test
		public void testEmployeeAddressCanBeChanged() {
			EmployeeManager manager = new EmployeeManager();
			Employee employee = new TestEmployee();
			int employeeID = manager.hireEmployee(employee);

			Address address1 = new TestAddress();
			manager.setAddress(employeeID, address1);
			assertEquals(address1, manager.getAddress(employeeID));

			Address address2 = new TestAddress();
			manager.setAddress(employeeID, address2);
			assertNotEquals(address1, manager.getAddress(employeeID));
			assertEquals(address2, manager.getAddress(employeeID));
		}

		@Test
		public void testEmployeePhoneNumberCanBeChanged() {
			EmployeeManager manager = new EmployeeManager();
			Employee employee = new TestEmployee();
			int employeeID = manager.hireEmployee(employee);

			String phone1 = "123456789";
			manager.setPhoneNumber(employeeID, phone1);
			assertEquals(phone1, manager.getPhoneNumber(employeeID));

			String phone2 = "987654321";
			manager.setPhoneNumber(employeeID, phone2);
			assertNotEquals(phone1, manager.getPhoneNumber(employeeID));
			assertEquals(phone2, manager.getPhoneNumber(employeeID));
		}

		@Test
		public void testEmployeeSalaryCanBeChanged() {
			EmployeeManager manager = new EmployeeManager();
			Employee employee = new TestEmployee();
			int employeeID = manager.hireEmployee(employee);

			double salary1 = 123;
			manager.setSalary(employeeID, salary1);
			assertEquals(salary1, manager.getSalary(employeeID), 0);

			double salary2 = 456;
			manager.setSalary(employeeID, salary2);
			assertNotEquals(salary1, manager.getSalary(employeeID));
			assertEquals(salary2, manager.getSalary(employeeID), 0);
		}

		@Test
		public void testEmployeeSalaryCanBeRaised() {
			EmployeeManager manager = new EmployeeManager();
			Employee employee = new TestEmployee();
			int employeeID = manager.hireEmployee(employee);

			manager.setSalary(employeeID, 123);
			manager.raiseSalary(employeeID, 321);
			assertEquals(444, manager.getSalary(employeeID), 0);
		}
	}

These tests, although reduced and providing a 100% line coverage, illustrate one of the problem we mentionned. Namely, the heavy test: although they remain reduced, you can notice that all the tests need to hire an employee before to do anything. While it is expected for tests about hiring and firing, it is a bit troublesome to depend on a working hiring process to test other features. And yet, this case is only illustrative: in practice, you may need to do many things before to have a properly set up EmployeeManager, for each test. You may factor the set up, but it only remove the repeating, it does not remove the heavy dependency. You may use mocking features to directly set the private field EmployeeManager.employees, but your tests would depend on internal details and break after some code refactoring. After removing the God object (or replacing it by a façade), these compromises won't be needed anymore.