4장 - 타입 코드 처리하기 [긴 if 문 리팩터링]

긴 if 문 리팩터링

drawMap 메서드는 '다섯줄 제한 규칙'과 '함수 시작에만 if 문 배치' 규칙을 위반한다.

function drawMap(g: CanvasRenderingContext2D) {
	for (let y = 0; y < map.length; y++) {
		for (let x = 0; x < map[y].length; x++) {
			if (map[y][x] === Tile.FLUX) g.fillStyle = "#ccffcc";
			else if (map[y][x] === Tile.UNBREAKABLE) g.fillStyle = "#999999";
			else if (map[y][x] === Tile.STONE || map[y][x] === Tile.FALLING_STONE)
				g.fillStyle = "#0000cc";
			else if (map[y][x] === Tile.BOX || map[y][x] === Tile.FALLING_BOX)
				g.fillStyle = "#8b4513";
			else if (map[y][x] === Tile.KEY1 || map[y][x] === Tile.LOCK1)
				g.fillStyle = "#ffcc00";
			else if (map[y][x] === Tile.KEY2 || map[y][x] === Tile.LOCK2)
				g.fillStyle = "#00ccff";

			if (map[y][x] !== Tile.AIR && map[y][x] !== Tile.PLAYER)
				g.fillRect(x * TILE_SIZE, y * TILE_SIZE, TILE_SIZE, TILE_SIZE);
		}
	}
}

if ~ else-if 부분을 메서드로 추출하면 다음과 같아진다.

function drawMap(g: CanvasRenderingContext2D) {
	for (let y = 0; y < map.length; y++) {
		for (let x = 0; x < map[y].length; x++) {
			colorOfTile(g, x, y);
			if (map[y][x] !== Tile.AIR && map[y][x] !== Tile.PLAYER)
				g.fillRect(x * TILE_SIZE, y * TILE_SIZE, TILE_SIZE, TILE_SIZE);
		}
	}
}

function colorOfTile(g: CanvasRenderingContext2D, x: number, y: number) {
	...
}

drawMap 메서드는 규칙을 지키지만 colorOfTile 메서드는 다섯줄 제한 규칙을 위반한다.

기존에 있던 Tile Eunm을 다음과 같이 인터페이스로 바꿔주고

// 기존
enum Tile {
  AIR,
  FLUX,
  UNBREAKABLE,
  PLAYER,
  STONE, FALLING_STONE,
  BOX, FALLING_BOX,
  KEY1, LOCK1,
  KEY2, LOCK2
}

// 리팩터링
interface Tile {
	isAir(): boolean;
	isFlux(): boolean;
	isUnbreakale(): boolean;
	isPlayer(): boolean;
	isStone(): boolean;
	isFallingStone(): boolean;
	isBox(): boolean;
	isFallingBox(): boolean;
	isKey1(): boolean;
	isLock1(): boolean;
	isKey2(): boolean;
	isLock2(): boolean;
}

Tile 인터페이스를 구현하는 클래스를 만들어준다.

class Air implements Tile {
	isAir(): boolean {
		return true;
	}
	isFlux(): boolean {
		return false;
	}
	isUnbreakale(): boolean {
		return false;
	}
    ...
}

그 후 Tile을 2차원 배열로 가지는 map 배열을 다음과 같이 바꿔준다.

// 기존
let map: Tile[][] = [
	[2, 2, 2, 2, 2, 2, 2, 2],
	[2, 3, 0, 1, 1, 2, 0, 2],
	[2, 4, 2, 6, 1, 2, 0, 2],
	[2, 8, 4, 1, 1, 2, 0, 2],
	[2, 4, 1, 1, 1, 9, 0, 2],
	[2, 2, 2, 2, 2, 2, 2, 2],
];

// 2 -> new Unbrekable()
// 3 -> new Player()
...

colorOfTile 메서드도 바꿔주자.

function colorOfTile(g: CanvasRenderingContext2D, x: number, y: number) {
	if (map[y][x] === Tile.isFlux()) g.fillStyle = "#ccffcc";
	else if (map[y][x] === Tile.isUnbreakale()) g.fillStyle = "#999999";
	else if (map[y][x] === Tile.isStone() || map[y][x] === Tile.isFallingStone())
		g.fillStyle = "#0000cc";
	else if (map[y][x] === Tile.isBox() || map[y][x] === Tile.isFallingBox())
		g.fillStyle = "#8b4513";
	else if (map[y][x] === Tile.isKey1() || map[y][x] === Tile.isLock1())
		g.fillStyle = "#ffcc00";
	else if (map[y][x] === Tile.isKey2() || map[y][x] === Tile.isLock2())
		g.fillStyle = "#00ccff";
}

 

메서드 전문화

class Lock1과 class Lock2 를 제거하는 remove 메서드는 다음과 같다.

function remove(tile: Tile) {
	for (let y = 0; y < map.length; y++) {
		for (let x = 0; x < map[y].length; x++) {
			if (map[y][x] === tile) {
				map[y][x] = Tile.AIR;
			}
		}
	}
}

remove 메서드는 Tile 인터페이스를 상속하는 어떤 Tile이든 삭제를 하는 일반성을 띄고있다.

remove 메서드를 잘못호출하여 모든 타일을 제거할 수도 있고, 이러한 일반성은 코드의 유연성이 떨어지며 코드를 변경하기 어렵게 만든다.

다음과 같이 remove메서드를 특정해보자.

function removeLock1(tile: Tile) {
	for (let y = 0; y < map.length; y++) {
		for (let x = 0; x < map[y].length; x++) {
			if (map[y][x].isLock1()) {
				map[y][x] = new Air();
			}
		}
	}
}

function removeLock2(tile: Tile) {
	for (let y = 0; y < map.length; y++) {
		for (let x = 0; x < map[y].length; x++) {
			if (map[y][x].isLock2()) {
				map[y][x] = new Air();
			}
		}
	}
}

이런 리팩터링은 중복된 코드로 인해 오히려 리팩터링 효과가 떨어 질 수 있다는 생각을 들게한다. (완전 공감)

하지만 책에서는 일반화하고 재사용하려는 코드는 책임이 흐려지고 다양한 위치에서 코드를 호출할 수 있기 때문에 문제가 될 수 있고, 전문화된 메서드는 더 적은 위치에서 호출되어 필요성이 없어지면 더 빨리 제거할 수 있다고한다.

 

if-else 제거

colorOfTile 메서드는 여전히 if-else 문을 가지고 있다.

colorOfTile 메서드는 각 타일의 종류에 따라 색상을 부여하는 역할을 하고 있으므로, 각 타일의 return값을 boolean에서 색상값으로 바꿔준다면 코드는 더욱 간결해진다.

다음 코드를 보자.

interface Tile {
	color(g: CanvasRenderingContext2D): void;
}

class Flux implements Tile {
	color(g: CanvasRenderingContext2D): void {
		g.fillStyle = "#ccffcc";
	}
}

...

Tile 인터페이스는 타일의 색상을 변하는 메서드를 구현하게 하고 각 타일은 타일에 맞는 색상을 변경하도록 한다.

이렇게 되면 colorOfTile 메서드는 필요없어지고 drawMap 메서드는 다음과 같아진다.

function drawMap(g: CanvasRenderingContext2D) {
	for (let y = 0; y < map.length; y++) {
		for (let x = 0; x < map[y].length; x++) {
			map[y][x].color(g); // 요 부분이 변경되었다
			if (map[y][x] !== Tile.AIR && map[y][x] !== Tile.PLAYER)
				g.fillRect(x * TILE_SIZE, y * TILE_SIZE, TILE_SIZE, TILE_SIZE);
		}
	}
}

그래도 여전히 if문이 함수 맨 앞에 나오지 않고 있는데 if문을 Tile 인터페이스로 이관하면 규칙을 지킬 수 있다.

interface Tile {
	color(g: CanvasRenderingContext2D, x: number, y: number): void;
}

class Flux implements Tile {
	color(g: CanvasRenderingContext2D, x: number, y: number): void {
		g.fillStyle = "#ccffcc";
		g.fillRect(x * TILE_SIZE, y * TILE_SIZE, TILE_SIZE, TILE_SIZE);
	}
}

class Air implements Tile {
	color(g: CanvasRenderingContext2D, x: number, y: number): void {
		g.fillStyle = "#ccffcc";
	}
}

...

function drawMap(g: CanvasRenderingContext2D) {
	for (let y = 0; y < map.length; y++) {
		for (let x = 0; x < map[y].length; x++) {
			map[y][x].color(g, x, y);
		}
	}
}

이쯤되면 인터페이스 대신에 추상 클래스를 사용할 수 없을까? 라는 생각이 든다.

물론 추상 클래스를 사용할 수 있지만 인터페이스를 사용할 때 장점인 '개발자에게 구현을 강제한다'라는 것 때문이라도 인터페이스를 사용하는 것이 더 좋다.

 

복잡한 if 체인 구문 리팩터링

moveHorizontal 메서드는 복잡한 if 체인 구문을 가지고 있다.

function moveHorizontal(dx: number) {
	if (
		map[playery][playerx + dx].isFlux() ||
		map[playery][playerx + dx].isAir()
	) {
		...
	} else if (
		(map[playery][playerx + dx].isStone() ||
			map[playery][playerx + dx].isBox()) &&
		map[playery][playerx + dx + dx].isAir() &&
		!map[playery + 1][playerx + dx].isAir()
	) {
		...
	}
    ...
}

 

isFlux()와 isAir()을 하나로 묶어 isEdible()로 사용하고, isStone()과 isBox()를 묶어 isPushable()하나로 사용하면 moveHorizontal 메서드는 다음과 같아진다.

interface Tile {
	...
	isEdible(): boolean;
	isPushable(): boolean;
}

class Air implements Tile {
	...
	isEdible(): boolean {
		return true;
	}
	isPushable(): boolean {
		return false;
	}
}

class Box implements Tile {
	...
	isEdible(): boolean {
		return false;
	}
	isPushable(): boolean {
		return true;
	}
}

function moveHorizontal(dx: number) {
	if (map[playery][playerx + dx].isEdible()) {
		...
	} else if (
		(map[playery][playerx + dx].isPushable() &&
		map[playery][playerx + dx + dx].isAir() &&
		!map[playery + 1][playerx + dx].isAir()
	) {
		...
	}
    ...
}

 

클래스로 코드 이관

각 타일 클래스는 타일 종류에 따라 움직이는 방법이 다르지만 그 형태는 비슷하다.

이때 움직이는 메서드를 타일 클래스로 이관 하면 코드는 다음과 같아진다.

먼저 Tile 인터페이스에 moveHorizontal 메서드를 추가하고

interface Tile {
	...
	moveHorizontal(dx: number):void;
}

타일의 종류에 맞게 moveHorizontal 메서드를 구현해주면

class Box implements Tile {
	...
	moveHorizontal(dx: number): void {
		if (map[playery][playerx + dx + dx].isAir() &&
		!map[playery + 1][playerx + dx].isAir()) {
			map[playery][playerx + dx + dx] = this;
		moveToTile(playerx + dx, playery);
		}
	}
}

...

function moveHorizontal(dx: number) {
	map[playerx][playerx + dx].moveHorizontal(dx);
}

moveHorizontal 메서드는 한 줄 이므로 메서드 인라인화가 가능하다.