[리팩터링] 4장 타입 코드 처리하기

이번 장에서 다룰 내용

• if 문에서 else를 사용하지 말것 switch를 사용하지 말 것 으로 이른 바인딩 제거하기
• 클래스로 타입 코드 대체와 클래스로의 코드 이관으로 if문 제거하기
• 메서드 전문화로 문제가 있는 일반성 제거하기
• 인터페이스에서만 상속받을 것으로 코드간 커플링(결합) 방지하기
• 메서드의 인라인화 및 삭제 후 컴파일하기를 통한 불필요한 메서드

이번 장 클래스 리팩터링 공부해서 회사에서 개발한 calculator class 개선하자!

앞 장의 끝에서 else if 문을 분리하고 싶지 않았기 때문에 메서드 추출로 추출할 수 없는 handleInput 함수를 만들었다. 안타깝게도 handleInput은 다섯 줄 제한 규칙을 준수하지 않아서 개선해야한다.

function handleInput(input: Input){
  if (input === Input.LEFT) moveHorizontal(-1);
  else if (input === Input.RIGHT) moveHorizontal(1);
  else if (input === Input.UP) moveHorizontal(-1);
  else if (input === Input.DOWN) moveHorizontal(1);
}

4.1 간단한 if 문 리팩터링

4.1.1 규칙: if 문에서 else를 사용하지 말 것

정의

프로그램에서 이해하지 못하는 타입(형)인지를 검사하지 않는 한 if 문에서 else 를 사용하지 말자.

설명

if-else 를 사용하면 코드에서 결정이 내려지는 지점을 고정하게 된다. 그럴 경우 if-else 가 있는 위치 이후에서는 다른 변경을 도입할 수 없기 때문에 코드의 유연성이 떨어진다.

if-else는 하드코딩된 결정으로 볼 수 있다. 코드에서 하드코딩된 상수가 좋지 않는 것처럼 하드코딩된 결정도 좋지 않다.

// 변경 전
function average(ar: number[]){
  if(size(ar) === 0)
    throw "Empty array not allowed";
  else
    return sum(ar) / size(ar);
}

// 변경 후
function assertNotEmpty(ar: number[]){
  if(size(ar) === 0)
    throw "Empty array not allowed";
}

function average(ar: number[]){
  assertNotEmpty(ar);
  return sum(ar) / size(ar);
}

스멜

이 규칙은 스멜로 인식되는 이른 바인딩(early binding)과 관련이 있다.
프로그램을 컴파일할 때 if-else 같은 의사결정 동작은 컴파일 시 처리되어 애플리케이션에 고정되며 재컴파일 없이는 수정할 수 없다. 이것의 반대는 코드가 실행되는 순간에 동작이 결정되는 늦은 바인딩(late binding) 이다.
이른 바인딩은 if문을 수정해야 변경할 수 있기 때문에 추가에 의한 변경을 방해한다.

4.1.2 규칙적용

handleInput에서 if-else를 제거하는 첫 번째 단계는 Input을 열겨형(enum)에서 인터페이스(interface)로 바꾸는것이다. 그러면 값들은 클래스로 바뀐다. 마지막으로, 값이 이제 객체가 되었기 때문에 if 구문 내의 코드를 각 클래스의 메서드로 옮길 수 있다.

1. 열거형에 있는 4개의 값에 대한 메서드와 함께 임시 명칭으로 Input2라는 새로운 인터페이스를 도입한다.

enum Input {
  RIGHT, LEFT, UP, DOWN
}

interface Input2 {
  isRight(): boolean;
  isLeft(): boolean;
  isUp(): boolean;
  isDown(): boolean;
}

2. 4개의 열거 값에 해당하는 4개의 클래스를 만든다. 클래스에 해당하는 메서드를 제외한 모든 메서드는 false를 반환해야한다.(임시 메서드)

// 새로운 클래스
class Right implements Input2 {
  isRight {return true;} // isRight는 Right 클래스에서 true를 반환
  isLeft {return false;}
  isUp() {return false;}
  isDown() {return false;}
}
class Left implement Input2 {...}
class Up implement Input2 {...}
class Down implement Input2 {...}

3. 열거형의 이름을 RawInput 같은 이름으로 바꾼다. 그러면 컴파일러가 이 열거형을 사용한 모든 위치에서 오류를 발생시킨다.

4. 매개변수의 타입을 Input에서 Input2로 변경하고 일치 여부 검사를 새로운 메서드로 대체한다.

//변경 전
function handleInput(input: Input){
  if(input === Input.LEFT)
    moveHorizontal(-1);
  else if (input === Input.RIGHT)
    moveHorizontal(1)
  else if (input === Input.UP)
    moveHorizontal(-1);
  else if (input === Input.DOWN)
    moveVertical(1);
}

//변경 후
function handleInput(input:Input2){
  if (input.isLeft())
    moveHorizontal(-1);
  else if (input.isRight())
    moveHorizontal(-1);
  else if (input.isUp())
    moveHorizontal(-1);
  else if (input.isDown())
    moveHorizontal(-1);
}

5. 변경으로 인한 오류 수정

6. 마지막으로 Input2의 이름을 모든 곳에서 Input으로 변경한다. 이 시점에서 코드는 다음과 같다.

// 변경 전
window.addEventListener("keydown", e => {
  if (e.key === LEFT_KEY || e.key === "a") inputs.push(Input.LEFT);
  else if (e.key === UP_KEY || e.key === "w") inputs.push(Input.UP);
  else if (e.key === RIGHT_KEY || e.key === "d") inputs.push(Input.RIGHT);
  else if (e.key === DOWN_KEY || e.key === "s") inputs.push(Input.DOWN);
  
function handleInput(input: Input){
  if(input === Input.LEFT)
    moveHorizontal(-1);
  else if (input === Input.RIGHT)
    moveHorizontal(1)
  else if (input === Input.UP)
    moveVertical(-1);
  else if (input === Input.DOWN)
    moveVertical(1);
}
  
// 변경 후
window.addEventListener("keydown", e => {
  if (e.key === LEFT_KEY || e.key === "a") inputs.push(new Left());
  else if (e.key === UP_KEY || e.key === "w") inputs.push(new Up());
  else if (e.key === RIGHT_KEY || e.key === "d") inputs.push(new Right());
  else if (e.key === DOWN_KEY || e.key === "s") inputs.push(new Down());  
});

function handleInput(input:Input){
  if (input.isLeft())
    moveHorizontal(-1);
  else if (input.isRight())
    moveHorizontal(-1);
  else if (input.isUp())
    moveVertical(-1);
  else if (input.isDown())
    moveVertical(1);
}

4.1.3 리팩터링 패턴: 클래스로 타입코드 대체

이 리팩터링 패턴은 열거형을 인터페이스로 변환하고 열거형의 값들은 클래스가 된다. 그렇게 하면 각 값에 속성을 추가하고 해당 특정 값과 관련된 기능을 특성에 맞게 만들 수 있다.
애플리케이션 전체에 걸쳐 switch 또는 else if 체인으로 열거형을 자주 사용하기 때문이다. switch는 열거형이 가지는 값들이 처리되어야 하는 방식을 한곳에 기술한 것이다.

• 열거형의 값을 클래스로 변환할 때는 다른 열거 값을 고려하지 않고 해당 값과 관련된 기능을 그룹화할 수 있다.
• 열거형에 새 값을 추가하는 것은 수많은 파일에 걸쳐서 해당 열거형과 연결된 로직들을 확인해야 하는 반면, 인터페이스를 구현한 새로운 클래스를 추가하는 것은 해당 클래스에 메서드의 구현이 필요할 뿐, 새로운 클래스를 사용하기 전까지는 다른 코드를 수정하지 않아도 된다.

정해진 상수를 참조하지 않고 숫자를 사용했을 수 있기 때문에 int의 경우 타입 코드의 사용을 추적하는것이 까다롭다. 따라서 타입 코드를 볼 때는 항상 열거형으로 변환한다. 그리야한 이 리팩터링 패턴을 안전하게 적용할 수 있다.

//변경 전
const SMALL = 33;
const MEDIUM = 37;
const LARGE = 42;

// 변경 후
enum TShirtSizes {
  SMLL = 33,
  MEDIUM = 37,
  LARGE = 42
}

절차

1. 임시 이름을 가진 새로운 인터페이스를 도입한다. 인터페이스에는 열거형(enum)의 각 값에 대한 메서드가 있어야한다.
2. 열거형의 각 값에 해당하는 클래스를 만든다. 클래스에 해당하는 메서드를 제외한 인터페이스의 모든 메서드는 false를 반환해야한다.
3. 열거형의 이름을 다른 이름으로 바꾼다. 그렇게 하면 컴파일러가 열거형을 사용하는 모든 위치에서 오류를 발생시킨다.
4. 타입을 이전 이름에서 임시 이름으로 변경하고 일치성 검사를 새로운 메서드로 대체한다.
5. 남아있는 열거형 값에 대한 참조 대신 새로운 클래스를 인스턴스화하여 교체한다.
6. 오류가 더 이상 없으면 인터페이스의 이름을 모든 위치에서 영구적인 것으로 바꾼다.

4.1.4 클래스로 코드 이관하기

handleInput의 모든 조건은 매개변수 input과 관련 있으며, 이는 코드가 해당 클래스에 있어야 함을 의미한다.

1. handleInput을 복사해서 모든 클래스에 붙여 넣는다. 이제 메서드이기 때문에 function 키워드를 제거하고 입력 매개변수를 this로 바꾼다.

class Right implements Input {
 handleInput(){ //function 과 매게변수 제거
  if (this.isLeft())
    moveHorizontal(-1);
  else if (this.isRight())
    moveHorizontal(-1);
  else if (this.isUp())
    moveVertical(-1);
  else if (this.isDown())
    moveVertical(1);
  }
}

2. 메서드 선언을 Input 인터페이스에 복사하고 원래 메서드 handleInput과 약간 다른 이름을 지정한다. 이 경우 Input이 이미 있으므로 두 번 쓰는 것은 의미가 없다.

interface Input {
  // ...
  handle() :void;
}

3. 네가지 클래스 모두에서 handleInput 메서드를 변경한다.

class Right implements Input {
  //...
  handle(){
    moveHorizontal(1);
  }
}

4. handleInput의 내용을 새로운 메서드에 대한 호출로 수정한다.

//변경 전
function handleInput(input:Input){
  if (input.isLeft())
    moveHorizontal(-1);
  else if (input.isRight())
    moveHorizontal(-1);
  else if (input.isUp())
    moveVertical(-1);
  else if (input.isDown())
    moveVertical(1);
}

//변경 후
function handleInput(input: Input){
  input.handle()
}

여러 절차를 거친 후 이렇게 멋잰 개선 코드를 얻었다. 모든 if 구문이 사라졌고 이 메서드는 다섯 줄 제한을 지키게 되었다.

//변경 전
function handleInput(input:Input){
  if (input.isLeft())
    moveHorizontal(-1);
  else if (input.isRight())
    moveHorizontal(-1);
  else if (input.isUp())
    moveVertical(-1);
  else if (input.isDown())
    moveVertical(1);
}

//변경 후
function handleInput(input: Input){
  input.handle();
}

interface Input {
  //...
  handle(): void;
}

class Left implements Input {
  // ...
  handle() {moveHorizontal(-1);}
}

class Right implements Input {
  // ...
  handle() {moveHorizontal(1);}
}
class Up implements Input {
  // ...
  handle() {moveVertiacl(-1);}
}
class Down implements Input {
  // ...
  handle() {moveVertical(1);}
}

이것은 클래스로의 코드 이관이다.

4.1.7 리팩터링 패턴: 메서드의 인라인화

설명

프로그램에서 더 이상 가독성에 도움이 되지 않는 메서드를 제거한다. 그 방법은 메서드에서 이를 호출하는 모든 곳으로 코드를 옮기는 것이다. 이렇게 하면 메서드가 사용되지 않으므로 안전하게 삭제할 수 있다.

절차

1. 메서드 이름을 임시로 변경한다. 그러면 함수를 사용하는 모든 곳에서 컴파일러 오류가 발생한다.
2. 메서드의 본문을 복사하고 매개변수를 기억해 둔다.
3. 컴파일러가 오류를 발생시킨 모든 곳의 호출을 복사한 코드로 교체하고 인자를 매개변수에 매핑한다.
4. 오류 없이 컴파일되면 원래의 메서드가 더이상 사용되지 않는 것이므로 원래 메서드를 삭제한다.

예제

계좌에서 돈을 인출해서 다른 계좌에 입금하는 은행 거래를 두부분으로 분할 했다. 이는 실수로 deposit메서드를 잘못 호출하면 출금 없이 돈을 임금할 수도 있다는 것을 의미한다. 이를 해결하기 위해 두 가지 메서드를 결합하기로 했다.

function deposit(to: string, amount:number) {
  let accountId = database.find(to);
  database.updateOne(accountId, {$inc: {balance: amount} });
}

function transfer(from: string, to:string, amount:number) {
  deposit(from, -amount);
  deposit(to, amount);
}

절차

1. 메서드의 이름을 임시로 변경한다. 그러면 이 함수를 사용하는 모든 곳에서 컴파일러 오류가 발생한다.

// 변경 전
function deposit(to:string, amount: number){
  //...
}

// 변경 후
function deposit2(to:string, amount:number){
  //...
}

2. 메서드 본문을 복사하고 매개변수를 기억해 둔다.
3. 컴파일러에서 오류가 발생하는 모든 곳에서 메서드 호출을 복사된 코드로 변경하고 인자를 매개변수에 매핑한다.

// 변경 전
function transfer(from: string to:string, amount: numer){
  deposit(from, -amount);
  deposit(to, amount);
}

// 변경 후
function transfer(from: string, to:string, amount: numer) {
  let fromAccountId = database.find(from);
   database.updateOne(fromAccountId, {$inc: {balance: -amount }});
  let toAccountId = database.find(to);
  database.updateOne(toAccountId, {$inc: {balance: amount }});
}

4. 컴파일러 오류가 없으면 원래 메서드가 더 이상 사용되지 않는다는 것을 의미한다. 따라서 원래 메서드를 삭제 한다.

이제 deposit 함수를 사용해 잘못 계산될 여지가 없어졌다. 이 코드 복제가 좋은 생각인지 아닌지는 논란의 여지가 있다.

4.2 긴 if 문의 리팩터링

drawMap 살펴보기

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 체인 구문

따라서 가장 먼저 할 일은 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){
   if (map[y][x] === title.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";
}

이제 drawMap은 다섯 줄 제한 규칙을 준수하므로 colorOfTile로 진행한다.
colorOfTile은 if문에서 else를 사용하지 말 것 규칙을 위반한다.
이 문제를 해결하기 위해 열거형(enum)인 Tile을 Tile 인터페이스로 바꾼다.

1.열거형의 모든 값에 대한 메서드와 함께 Tile2라는 임시 이름을 사용하는 새로운 인터페이스를 도입한다.

interface Tile2 {
  isFlux(): boolean;
  isUnbreakable(): boolean;
  isStone(): boolean;
  // ... 열거형의 모든 값에 대한 메서드
}

2. 열거형의 각 값에 해당하는 클래스를 생성한다.

class Flux implements Title2 {
  isFlux() {return true;}
  isUnbreakable() {return false;}
  isStone() {return false;}
}

class Unbreakable implements Title2 {...}
class Stone implements Title2 {...}

3. 열거형의 이름을 RawTile로 변겅항여 컴파일러 오류로 열거형이 사용되는 모든 위치를 찾을 수 있다.

4. 일치성 검사를 새로운 메서드로 변경한다. 어플리케이션 전반에 걸쳐 많은 곳에서 이 변경 작업을 해아한다.

//변경 전
function colorOfTile(g: CanvasRenderingContext2D, x:number ,y:number){
   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";
}

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

5. Tile.FLUX를 new Flux(), title.AIR를 new Air() 와 같은 식으로 바꾼다.

이시점에서 오류가 없다면 Tile2를 Title로 변경할 수 있었다.
여전히 Tile을 사용하고 있음을 알리는 오류가 두 곳 있다.

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],
];

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] = new Air();
      }
    }
  }
}

4.2.1 일반성 제거

remove 함수의 문제는 타일의 타입을 사용해 map의 모든 위치에서 주어진 타입의 타일을 제거하는데, Tile의 특정 인스턴스인지 확인하지 않고 유사한지만 확인한다.

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] = new Air();
      }
    }
  }
}

🚨 문제점
너무 일반적이다. 잘못하면 모든 타입의 타일을 제거할 수 도 있다.
이러한 일반성은 유연성이 떨어지고 변경하기 어렵게 만든다.
따라서 타입을 더 특정해야한다.

remove의 이름을 임시 이름인 remove2로 바꾼다. 네 군데에서 remove가 사용된다는것을 알 수 있다.

//변경 전
...
remove(new Lock1());
...
remove(new Lock2());
...
remove(new Lock1());
...
remove(new Lock2());

remove는 지정된 모든 유형의 타일을 제거하는것이 의도이지만, 실제로 Lock1또는 Lock2만 제거한다.

1. remove2를 복제한다.

//변경 전
function remove2(title: Tile){
  //...
}

//변경 후
function remove2(tile: Tile){
  //...
}
function remove2(title: Tile){
  //...
}

2. 둘 중 하나의 이름을 removeLock1 으로 바꾸고 해당 매개 변수를 제거한 후 임시로 === titld을 === Title.LOCK1으로 바꾼다. 이전에 처리했던 코드와 동일한 코드를 만들 수 있기 때문에 Title을 RawTile로 이름을 변경했음에도 이 변경 작업을 수행한다.

3. 이것은 정확히 제거할 타입이 LOCK1인지를 체크한다. 따라서 이전과 마찬가지로 메서드 호출로 변경한다.

//변경 전
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] === Title.LOCK1) {
        map[y][x] = new Air();
      }
    }
  }
}

//변경 후
function removeLock1 () {
  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();
      }
    }
  }
}

4. 이 함수에 더 이상 오류가 없으므로 새 함수를 호출하도록 이전 호출을 변경한다.

//변경 전
remove(new Lock1());

//변경 후
removeLock1();

5. removeLock2에 대해서도 동일한 작업을 수행한다.

일반화를 줄이고 좀 더 특정화한 버전의 함수를 도입하는 과정을 메서드 전문화 라고한다.

4.2.2 리팩터링 패턴: 메서드 전문화

설명

프로그래머들은 일반화하고 재사용하려는 본능적인 욕구가 있지만 그렇게 하면 책임이 흐려지고 다양한 위치에서 코드를 호출 할 수 있기 때문에 문제가 될 수 있다. 이 리팩터링 패턴은 이 효과를 반전 시킨다. 좀 더 전문화된 메서드는 더 적은 위치에서 호출되어 필요성이 없어지면 더 빨리 제거할 수 있다.

절차

1. 전문화 하려는 메서드를 복제한다.
2. 메서드 중 하나의 이름을 새로 사용할 메서드의 이름으로 변경하고 전문화하려는 매개변수를 제거(또는 교체)한다.
3. 매개변수 제거에 따라 메서드를 수정해서 오류가 없도록 한다.
4. 이전의 호출을 새로운 것을 사용하도록 변경한다.

4.2.4 규칙: switch를 사용하지 말 것

정의

default 케이스가 없고 모든 case에 반환 값이 있는 경우가 아니라면 switch를 사용하지 말것

설명

switch는 버그로 이어지는 두 가지 편의성을 허용하기 때문에 문제가 있다.

• switch는 default키워드를 지원한다. default로 중복 없이 여러 값을 지정할 수 있다. switch를 사용할 경우 무엇을 처리할지와 무엇을 처리하지 않을지 불변속성이다.
  그러나 기본값이 지정된 경우 새로운 값을 추가할 때 이러한 불변속성이 여전히 유효한지 컴파일러를 통해 판단할 수 없게 된다.
  컴파일러 입장에서는 새로 추가한 값의 처리를 잊어버린 것인지, default에 지정하고자 한 것인지 구분할 방법이 없다.

• switch는 break 키워드를 만나기 전 까지 케이스를 연속해서 실행하는 폴스루(fall-through)로직이다. switch를 사용할 때 break 키워드를 쓰는 것을 알아채지 못하기 쉽다.

일반적으로 switch는 멀리하는 것이 좋다.

문제점을 고치는 첫번째 방법은 기능을 default에 두지 않는것이다. 사용언어가 default의 생략을 허용하지 않으면 switch를 사용하면 안된다.

모든 케이스에 return을 지정해서 폴스루 문제를 해결한다. 결과적ㅇ로 폴스루 로직이 없어짐으로써 break를 잊어버려 문제가 발생할 여지가 없어진다.

스멜

switch는 스멜의 이름이다.
switch는 컨텍스를, 즉 값 X를 처리하는 방법에 초점을 맞춘다. 반대로, 클래스에 기능을 밀어 넣을 떄는 데이터 즉, 이 값(객체)이 상황 X를 처리하는 방법에 초점을 맞춘다. 컨텍스트에 초점을 맞춘다는 것은 데이터에서 불변속성을 더 멀리 위치시켜 불변속성을 전여고하 하는것을 의미한다.

의도

이 규칙의 흠이라면, switch를 else if 체인문으로 변환하고 이를 다시 클래스로 만든다는 것이다. 클래스로 이관하면서 if문들을 제거한다.
이 방법을 통해 결과적으로 기능을 유지하면서 새로운 값을 더 쉽고 안전하게 추가할 수 있다.

4.2.5 if 제거하기

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

🚨 colorOfTile은 if문에서 else를 사용하지 말 것 규칙을 위반한다.
클래스로의 코드 이관 패턴을 적용한다.

1. colorOfTile을 복사해서 모든 클래스에 붙여 넣는다. function 키워드를 제거한다. 이 경우 매개변수 y와 x를 제거하고 map[y][x]를 this로 바꾼다.

2. 메서드 선언부를 Tile 인터페이스에 복사한다. 이름도 color로 변경한다.

3. 모든 클래스에서 새로운 메서드를 살펴본다.
   a. is로 시작하는 메서드들을 인라인화한다.
   b. if(true) 및 if(false){...} 를 제거한다. 대부분의 새로운 메서드는 한 줄만 남고 Air 및 Player는 비어있다.
   c. 이름을 color로 변경하여 이 메서드가 완성되었음을 알린다.

4. colorOfTile의 본문을 map[y][x].color의 호출로 바꾼다.

이제 if가 사라져 더 이상 규칙을 위반하지 않게 된다.

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

// 변경 후 

function colorOfTile(g: CanvasRenderingContext2D, x:number ,y:number){
   map[y][x].color(g);
}
interface Tile {
  //...
  color: (g: CanvasRenderingContext2D):void
}
class Air implements Tile {
  // ...
  color(g: CanvasRenderingContext2D) {
    //모든 if 가 false이기 때문에 color는 Air 와 Player에서 비어있음
  }
}
  
class Flux implements Tile {
  // ...
  color(g: CanvasRenderingContext2D){
    g.fillStyle = "#ccffcc';
  }
}

colorOfTile이 한 줄이기 때무에 메서드의 인라인화로 수행하기로 한다.

//변경 전
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 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);
      }
    }
}

drawMap에서 긴 if 문을 제거했지만 여전히 규칙을 준수하지 않는 부분이 있어 계속 리팩터링을 진행한다!

4.3 코드 중복 처리

drawMap의 중간에 if가 존재하기 때문에 여전히 규칙을 위반한다. 여러 번 했던 것처럼 if 문을 추출함으로써 이를 해결할 수 있다.

// 변경 전
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);
      }
    }
}

// 변경 후
function drawMap(g: CanvasRenderingContext2D){
    for (let y = 0; y < map.length; y++) {
      for (let x = 0; x < map[y].length; x++) {
      	drawTile(g,y,x)
    }
}
  
function drawTile(g: CanvasRenderingContext2D, y:number, x:number){
   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);
      }
}

클래스로의 코드 이관 패턴을 사용해 이 메서드를 Tile클래스로 옮긴다.

// 변경 전
function drawTile(g: CanvasRenderingContext2D, y:number, x:number){
   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);
      }
}

// 변경 후
function drawTile(g: CanvasRenderingContext2D, y:number, x:number){
   map[y][x].draw(g, x, y) 
}

interface Tile {
  //...
  draw(g: CanvasRenderingContext2D, y:number, x:number): void;
}
class Air implements Tile {
  // ...
  draw(g: CanvasRenderingContext2D,y:number, x:number) {
    //draw는 Air 와 Player에서 비어있음
  }
}
  
class Flux implements Tile {
  // ...
  draw(g: CanvasRenderingContext2D,x:number,y:number){
    g.fillStyle = "#ccffcc';
    g.fillRect(x * TILE_SIZE, y * TILE_SIZE, TILE_SIZE, TILE_SIZE);
  }
  // 다른 모든 클래스는 color 및 isAir를 인라인화하고 if(true)를 제거하면 결국 두 줄이 됨
}

drawTitle 인라인

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

🙀 의문
클래스들에 있는 코드 중복 어저지... 인터페이스 대신 추상 클래스를 사용하고 거기에 모든 공통 코드를 넣을 수 없을까? 라는 의문에 대해 이야기 해보자!

4.3.1 인터페이스 대신 추상 클래스를 사용할 수는 없을까?

답은 '사용할 수 있다' 이다. 이렇게 하면 코드 중복을 피할 수 있다.
하지만 이 접근 방식은 중요한 단점도 있다.

인터페이스를 사용하면 이를 통해 도입한 각각의 새로운 클래스에 대해 개발자는 능동적으로 무엇을 해야한다.
따라서 잘못해서 속성을 잊어버리거나 해서는 안 되는 오버라이드(재정의)를 방지할 수 있다.
이것은 시간이 흐른 후 새로운 Tile의 유형을 추가해야 할 때 문제가 된다.

4.3.2 규칙: 인터페이스에서만 상속 받을 것

정의

상속은 오직 인터페이스를 통해서만 받는다.

설명

추상클래스를 사용하는 가장 일반적인 이유는 일부 메서드에는 기본 구현을 제공하고 다른 메서드는 추상화 하기 위한 것이다. 이것은 중복을 줄이고 코드의 줄을 줄이고자 할 경우 편리하다.

그러나 단점이 훨씬 많다.
코드 공유는 커플링(결합)을 유발한다.

자세한 예시와 설명
추상 클래스에 methodA와 methodBㄹ라는 두 가지 메서드가 구현되어 있다. 한 하위 클래스에는 methodA만 필요하고 다른 하위 클래스는 methodB만 있으면 될 경우 유일한 옵션은 메서드 중 하나를 빈 버전으로 제 정의하는 것이다.

기본 구현된 메서드가 있는 경우 다음과 같은 두가지 시나리오가 있다.

가능한 모든 하위 클래스에 해당 메서드가 필요한 경우인데, 이 경우는 메서드를 클래스 밖으로 쉽게 이동할 수 있다. 또는 일부 하위 클래스에서 메서드를 재정의해야하는 경우인데, 기본 구현이 있어 컴파일러를 통해 재정의가 필요한 메서드인지 잡아낼 수 없다. -> 기본값의 문제..

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

function moveHorizontal(dx: number) {
  if (map[playery][playerx + dx] === Tile.FLUX
    || map[playery][playerx + dx] === Tile.AIR) {
    moveToTile(playerx + dx, playery);
  } else if ((map[playery][playerx + dx] === Tile.STONE
    || map[playery][playerx + dx] === Tile.BOX)
    && map[playery][playerx + dx + dx] === Tile.AIR
    && map[playery + 1][playerx + dx] !== Tile.AIR) {
    map[playery][playerx + dx + dx] = map[playery][playerx + dx];
    moveToTile(playerx + dx, playery);
  } else if (map[playery][playerx + dx] === Tile.KEY1) {
    remove(Tile.LOCK1);
    moveToTile(playerx + dx, playery);
  } else if (map[playery][playerx + dx] === Tile.KEY2) {
    remove(Tile.LOCK2);
    moveToTile(playerx + dx, playery);
  }
}

우선 두 개의 || 표현식이 있다는 점에 주목하자. 이것들은 기본적인 도메인에 대한 어떤 의미를 가진다. 따라서 이 구조를 보존하면서 강조하고자한다. 해당 부분만 클래스로 옮김으로써 그렇게 할 수 있다.

이 접근 방식은 메서드 전체를 옮기는 것이 아니기 때문에 이전에 수행했던 방식과 약간 다르지만, 프로세스는 동일하다. 가장 어려운 부분은 좋은 이름을 짓는 것이다.

플럭스(Flux)와 공기(Air) 사이에 유사성이 있다고 정의하고자한다. 이것은 게임의 도메인에 관련된 것으로서 edible(먹을 수 있는 것)이라고 한다.

1. Tile 인터페이스에 isEdible 메서드를 새로 만든다.
2. 각 클래스에 이 메서드를 추가하는데, 이름을 임시로 약간 변형해서 isEdible2라고 한다.
3. 본문에 return this.isFlux() }|| this.isAir(); 를 입력한다.
4. isFlux와 isAir 값을 인라인화 한다.
5. 이름에서 임시로 넣은 2를 제거한다.
6. 여기서만 map[playery][playerx + dx].isFlux() || map[playery][playerx + dx].isAir()를 바꾼다. 다른 || 연산 절에서도 동일한 속성 (즉, edible)을 참조하는지는 모르기 때문에 모든 곳을 바꿀 수는 없다.

다른 || 표현식도 마찬가지 이다. 예를 들어 상자(Box)와 돌(Stone)은 동일한 맥락에서 밀 수 있는 (pushable) 특성을 가진다.

function moveHorizontal(dx: number) {
  if (map[playery][playerx + dx].isEdible()) {
    moveToTile(playerx + dx, playery);
  } else if (map[playery][playerx + dx].isPushable()
    && map[playery][playerx + dx + dx] === Tile.AIR
    && map[playery + 1][playerx + dx] !== Tile.AIR) {
    map[playery][playerx + dx + dx] = map[playery][playerx + dx];
    moveToTile(playerx + dx, playery);
  } else if (map[playery][playerx + dx] === Tile.KEY1) {
    remove(Tile.LOCK1);
    moveToTile(playerx + dx, playery);
  } else if (map[playery][playerx + dx] === Tile.KEY2) {
    remove(Tile.LOCK2);
    moveToTile(playerx + dx, playery);
  }
}

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

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

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

이 코드의 문맥에서 모든 if 절에 map[playerx + dx] 가 사용된다. 여기서 클래스로의 코드 이관이 일련의 일치성 검사로 시작할 때 뿐만 아니라 명확한 컨텍스트가 있는 모든 것, 즉 동일한 인스턴스(왼쪽이 동일한 모든[.] 표현) 에 대한 많은 메서드의 호출에도 적용된다는것을 알 수 있다.

따라서 코드를 map[playery][playerx + dx]; 즉 Tile로 옮긴다. 클래스로의 코드 이관 패턴을 적용한 후의 코드는 다음과 같다.

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

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

class Box implements Tile {
  // 박스(Box)와 돌(Stone)은 유사함
  //...
  moveHorizontal(dx:number){
    if(map[playery][playerx + dx + dx].isAir() && !map[playery + 1][playerx + dx].isAir()){
      map[playery][playerx + dx + dx] = this;
      moveToTile(playerx + dx, playery)
    }
  }
}

class Key1 implements Tile {
  // Key1 과 Key2는 유사함
  //...
  moveHorizontal(dx:number){
    removeLock1();
    moveToTile(playerx + dx, playery);               
  }
}

class Lock1 implements Tile {
  //나머지는 비어있음
  //...
  moveHorizontal(dx:number){}
}

class Air implements Tile {
  moveHorizontal(dx:number){
    moveToTile(playerx + dx, playery);
  }
}

moveHorizontal 메서드가 단 한줄이므로 인라인화 한다.

4.5 필요 없는 코드 제거하기.

비주얼 스튜디오 코드를 비롯한 많은 IDE는 사용되지 않는 함수를 표시한다. 이러한 표시가 있을 때 어떤 절차를 진행 중이 아니라면 즉시 함수를 삭제해야한다.

메서드를 삭제하고 컴파일러에서 허용하는지 확인하기

요고는 간단하게 정리하고 패스~

요약

• if 문에서 else를 사용하지 말것. 그리고 switch를 사용하지 말것 규칙에 따르면 else또는 switch는 프로그램의 가장자리에만 있어야 한다. else와 switch 모드 낮은 수준의 제어 흐름 연산자이다. 애플리케이션의 핵심에서는 클래스로 타입 코드 대체 및 클래스로의 코드 이관 리팩터링 패턴을 사용해서 switch와 연속된 else if 구문을 높은 수준의 클래스와 메서드로 대체해야한다.

• 지나치게 일반화된 메서드는 리팩터링을 방해 할 수 있다. 이런 경우 불필요한 일반성을 제거학 위해 메서드 전문화 리팩터링 패턴을 사용할 수 있따.

• 인터페이스에서만 상속받을 것 규칙은 추상 클래스와 클래스 상속을 사용해 코드를 재사용 하는것을 방지한다. 이러한 유형의 상속은 불필요하게 긴밀한 커플링을 발생 시킨다.

• 메서드의 인라인화, 삭제후 컴파일하기로 더 이상 가독성에 도움이 되지 않는 메서드를 제거할 수 있다.