One of the earliest, most fundamental things I learned about writing code was the Don't Repeat Yourself (DRY) principle.
Initially, this resonated with me. When writing the code, you've already solved a problem, why would you go to the effort of solving it a second time? While technically my understanding was correct, it only scratched the surface.
What is the DRY principle?
In The Pragmatic Programmer, Andrew Hunt and Dave Thomas discuss the "evils of duplication" in code. They note that code changes constantly: whether requirements change, business logic changes, or the person writing the code misunderstood or made a mistake (as crafters we are constantly changing our code).
As such:
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system
At its core, the DRY principle states that we should only have the code that does something in one place, and when we eventually need to modify the code, we will only need to change it a single time in a single place.
Let's look at an example
Two of my apprentices were recently pairing on their first JavaScript project; writing a human vs human game of Tic Tac Toe that can be played in the browser. They had TDD'd the logic, with a Game class with a takeTurn method that accepts the user's chosen position (between 1 and 9) as an argument, and marks the board with the that player's marker.
The initial attempt to connect the user's mouse click in the browser to their Game class was to create an event listener on each of the divs that represented a square in the grid, which had an id of the position number. When a user selects a square, they trigger an anonymous function which marks the board and displays the 'X' or the 'O' in the browser.
const game = new Game([1, 2, 3, 4, 5, 6, 7, 8, 9]);
document.getElementById('1').addEventListener('click', () => {
game.takeTurn(1);
two.innerHTML = game.showGrid()[0];
});
document.getElementById('2').addEventListener('click', () => {
game.takeTurn(2);
two.innerHTML = game.showGrid()[1];
});
...
document.getElementById('9').addEventListener('click', () => {
game.takeTurn(9);
two.innerHTML = game.showGrid()[8];
});
This works; however, their next feature was to display the current player on the screen, which required nine updates.
const displayPlayer = () => {
document.getElementById('current-player').innerHTML = `${game.currentPlayer}'s Turn`
}
document.getElementById('1').addEventListener('click', () => {
game.takeTurn(1);
two.innerHTML = game.showGrid()[0];
displayPlayer();
});
document.getElementById('2').addEventListener('click', () => {
game.takeTurn(2);
two.innerHTML = game.showGrid()[1];
displayPlayer();
});
...
document.getElementById('9').addEventListener('click', () => {
game.takeTurn(9);
two.innerHTML = game.showGrid()[8];
displayPlayer();
});
As you can imagine, making a change in nine places is far from ideal—and what if this is not the only place this code is referenced and we forget one?
My apprentices and I discussed some potential ways to solve the repetition problem. The solution we decided on was to move the event listener to the grid that contained the squares—rather than an event listener for each individual square. We get the user's choice based on the id of the square selected.
const chooseCell = (event) => {
const radix = 10;
const cellId = event.target.id;
const cellIdNumber = parseInt(cellId, radix);
game.takeTurn(cellIdNumber);
document.getElementById(cellId).innerHTML = game.showGrid()[cellIdNumber - offset];
displayPlayer();
}
document.getElementById('grid').addEventListener('click', chooseCell);
Now, if we want to make any changes to this logic, or extend the code, we don't need to change nine functions, we can change it in one place.
The next feature was to to notify the players of a winner, we created a displayEndOfGame method, and only had to reference it in one place rather than nine.
This change saves time while writing the code, but more importantly it saves time and is clearer to understand while reading the code.
What about in tests?
Tests are often overlooked when we are refactoring; however, our tests are the lifeblood and documentation of our code.
Let's look at some tests in our Game class with repetition:
describe('Game', () => {
it('marks the board when player one makes a move', () => {
const board = new Board([1, 2, 3, 4, 5, 6, 7, 8, 9]);
const game = new Game(board);
game.takeTurn(3);
expect(game.showBoard()).toEqual([1, 2, 'X', 4, 5, 6, 7, 8, 9]);
});
it('does not mark the board when all moves are exhausted', () => {
const board = new Board(['X', 'O', 'X', 'O', 'O', 'X', 'X', 'X', 'O'])
const game = new Game(board);
game.takeTurn(1);
expect(game.showBoard()).toEqual(['X', 'O', 'X', 'O', 'O', 'X', 'X', 'X', 'O']);
});
});
Creating a new instance of the board and game in each test is a smell. What if we change the signature of either of these objects?
A popular option to reduce this duplication could be to set them up in a beforeEach block. If we did this we'd rewrite these tests as:
describe('Game', () => {
let game;
beforeEach(() => {
const board = new Board([1, 2, 3, 4, 5, 6, 7, 8, 9]);
game = new Game(board);
});
it('marks the board when player one makes a move', () => {
game.takeTurn(3);
expect(game.showBoard()).toEqual([1, 2, 'X', 4, 5, 6, 7, 8, 9]);
});
it('does not mark the board when all moves are exhausted', () => {
game.takeTurn(3);
game.takeTurn(5);
game.takeTurn(1);
game.takeTurn(2);
game.takeTurn(8);
game.takeTurn(4);
game.takeTurn(6);
game.takeTurn(9);
game.takeTurn(7);
game.takeTurn(1);
expect(game.showBoard()).toEqual(['X', 'O', 'X', 'O', 'O', 'X', 'X', 'X', 'O']);
});
});
This works, and removes the need to instantiate the board and game each time; however, we lose the benefit of being able to pass in a marked board, which kept the tests readable.
My preference in a situation like this would be to create a helper method that takes the board state as an argument, and use that to set up the game.
describe('Game', () => {
const setUpGame = (boardState) => {
const board = new Board(boardState);
return new Game(board);
};
it('marks the board when player one makes a move', () => {
const game = setUpGame([1, 2, 3, 4, 5, 6, 7, 8, 9]);
game.takeTurn(3);
expect(game.showBoard()).toEqual([1, 2, 'X', 4, 5, 6, 7, 8, 9]);
});
it('marks the board when player two makes a move', () => {
const game = setUpGame(['X', 'O', 'X', 'O', 'O', 'X', 'X', 'X', 'O']);
game.takeTurn(1);
expect(game.showBoard()).toEqual(['X', 'O', 'X', 'O', 'O', 'X', 'X', 'X', 'O']);
});
});
This enables us to pass in a marked board to our setup method, eliminating the need to manually set up each move, while still removing the duplication in the setup of the game.
In summary, I find that adhering to the DRY principle while writing code not only keeps you from reinventing the wheel, but makes your code significantly more readable and maintainable in the future. It can occasionally take more time to write code in a way that avoids duplication up front; but I believe this initial investment is absolutely worth it - you will thank yourself for this foresight when you are reading and maintaining your code in the future - and your team will thank you too!