[리팩터링] 3장 긴코드 조각내기

공효은·2023년 5월 1일
typescript리팩터링
0

refactoring

목록 보기
3/12
post-thumbnail

이번 장에서 다룰 내용

  • 다섯 줄 제한(Five Lines)으로 지나치게 긴 메서드 식별하기
  • 세부 사항을 보지 않고 코드 작업하기
  • 메서드 추출(Extract method)로 긴 메서드 분해하기
  • 호출 또는 전달, 한 가지만 할것(Either Call Or Pass)으로 추상화 수준 맞추기
  • if 문은 함수의 시작에만 배치로 if 문 분리하기

DRY(Don't Repeat Yourself, 똑같은 일을 두 번 하지 말 것), 그리고 KISS(Keep It Simple, Stupid, 단순함을 지킬 것) 지침을 따른 경우라도 코드는 쉽게 지저분해지고 혼란 스러워질 수 있다. 이 혼란의 주 원인은 다음과 같다.

  • 메서드가 여러 가지 다른 일을 수행한다.
  • 낮은 수준의 원시 연산(배열 조작, 산술 연산 등)을 사용한다.
  • 주석과 적절한 메서드와 변수명 같이 사람이 읽을 수 있는 텍스트가 부족하다

3.1 첫 번째 규칙: 왜 다섯 줄인가?

3.1.1 규칙: 다섯 줄 제한

정의

메서드는 {와 }를 제외하고 5줄 이상이 되어서는 안된다.

설명

문장이라고 하는 코드 한줄은 하나의 if, for, while 또는 세미콜론으로 끝나는 모든 것을 말한다. 즉, 할당, 메서드 호출, return 같은 것이다. 공백과 중괄호({및})는 제외한다.

모든 메서드를 이 규칙을 준수하도록 바꿀 수 있습니다.
20줄의 메서드가 있을 경우 첫 10줄과 나머지10줄로 각각 도우미 메서드를 만든다. 원래의 메서드는 이제 두 줄이다. 한줄은 첫 번째 도우미를 호출하고 다른 한 줄은 두 번째 도우미를 호출한다.

특정 수치로 줄 수를 제한하는 것보다 제한이 있다는 것 자체가 중요하다.

이 책에서는 2차원 데이터로 작업한다. 즉, 기본 데이터 구조가 2차원 배열이다. 다음 두 함수는 2차원 배열을 탐색한다. 각각 정확히 다섯 줄의 코드이다.

// 2차원 배열에 짝수가 존재하는지 확인하는 함수
function containsEven(arr: number[][]){
  for (let x = 0; x < arr.length; x ++) {
    for (let y = 0; y < arr[x].length; y++) {
      if (arr[x][y] % 2 === 0 ) {
        return true;
      }
    }
  }
  return false;
}


// 2차원 배열에서 최소 항목을 찾는 함수
function minimum(arr :number[][]) {
  let result = Number.POSITIVE_INFINITY;
  for(let x = 0; x < arr.length; x++) {
    for (let y = 0; y < arr[x].length; y++) {
      result = Math.min(arr[x][y], result)
    }
  }
}

타입스크립트에서는...
변수를 선언할 때는 let 키워드를 사용합니다. let은 타입을 유추해야 하지만, let a:number = 5; 같이 지정할 수도 있습니다. 변수를 사용한 후 형을 지정할 수 있는 이상한 범위(scope) 규칙 때문에 var 키워드는 사용하지 않습니다.
다음의 왼쪽에 있는 코드는 유효하기는 하지만 의도한 바는 아닐 것이다. 

//나쁜 코드
a = 5;
var a:number;
//좋은 코드
a = 5;
let a:number;
//2장에서 본 4줄 메서드
function isTrue(bool:boolean){
  if(bool)
    return true;
  else return false;
}

스멜

메서드가 길다는 것 자체가 스멜이다. 그렇다면 길다는것은 무엇일까.
이 질문에 답하기 위해 메서드는 한 가지 작업만 해야 한다는 다른 스멜을 생각해볼 수 있다.
다섯 줄 제한이 하나의 의미있는 작업에 딱 맞는 크기라면 이 제한 역시 다른 스멜의 제거를 방해할 수 있다. 우리는 종종 코드에서 위치마다 기본 데이터 구조가 다른 환경에서 작업한다. 이 규칙에 익숙해지면 특정 사례에 맞게 줄 수를 변경할 수는 있겠지만, 실제로 줄의 수는 5줄 정도로 끝나는 경우가 많다.

각각 5줄의 코드가 있는 4개의 메서드가 20줄인 하나의 메서드보가 훨씬 빠르고 이해하기 쉽다. 각 메서드의 이름으로 코드의 의도를 전달할 수 있기 때문이다.

3.2 함수 분해를 위한 리팩터링 패턴 소개

  • 코드를 이해하기 위한 첫 번째 단계는 항상 함수명을 고려하는 것이다. 여기서는 코드의 '형태'를 살펴보는 것으로 시작한다.

  • 동일한 작업을 하는 데 필요한 줄의 그룹을 식별 해보자. 이러한 그룹을 명확히 하기 위해 그룹이라고 생각하는 곳에 빈줄을 추가한다. 때떄로 그룹과 관련된 것을 기억하는 데 도움이 되는 주석을 추가한다.(이 경우 주석은 일시적인 것이다)
    함수 전체를 소화하려고 하지 말고, 작게 잘라서 이해하기 쉽게 하나씩 처리한다.

3.2.1 리팩터링 패턴: 메서드 추출

설명

메서드 추출은 한 메서드의 일부를 취해서 자체 메서드로 추출한다.
여러 매개변수가 필요하거나 여러 조건에서 전체가 아닌 일부 조건에서만 return할 경우 복잡질 수 있다. 일반적으로 메서드에서 줄을 재정렬 하거나 복제해서 단순화 할 수 있다.

절차

  1. 추출할 줄의 주변을 빈 줄로 표시하는데, 주석으로 표시할 수도 있다.
  2. 원하는 이름으로 새로운 빈 메서드를 만든다.
  3. 그룹의 맨 위에서 새로운 메서드를 호출한다.
  4. 그룹의 모든 줄을 선택해서 잘라내어 새로운 메서드의 본문에 붙여넣는다.
  5. 컴파일 한다.
  6. 매개변수를 도입하여 호출하는 쪽의 오류를 발생시킨다.
  7. 이러한 매개변수 중 하나(p)를 반환 값으로 할당해야 할 경우
    a 새로운 메서드의 마지막에 return p;를 추가한다.
    b 새로운 메서드를 호출하는 쪽에서 p = newMethod(...)와 같이 반환 값을 할당한다.
  8. 컴파일한다.
  9. 호출 시 인자를 전달해서 오류를 잡는다.
  10. 사용하지 않는 빈 주석을 제거한다.

예제

2차원 배열에서 최소 항목을 찾는 함수가 길다고 생각하여 쪼갤려고 한다.

//변경 전
function minimum(arr :number[][]) {
  let result = Number.POSITIVE_INFINITY;
  for(let x = 0; x < arr.length; x++) {
    for (let y = 0; y < arr[x].length; y++) {
      result = Math.min(arr[x][y], result)
    }
  }
}

//변경 후
function minimum(arr :number[][]) {
  let result = Number.POSITIVE_INFINITY;
  for(let x = 0; x < arr.length; x++) {
    for (let y = 0; y < arr[x].length; y++) {
      result = min(result, arr, x, y);
    }
   return result;
  }
}

function min(result: number, arr:number[][], x:number, y:number){
  if(result > arr[x][y])
    result = arr[x][y];
  return result;
}

흠... 읽기 더 힘든거같기도하고..

자신감이 떨어지는 예쁜 코드를 만드는 것보다 특이하게 생긴 안전한 코드를 만드는 편이 낫다.

3.3 추상화 수준을 맞추기 위한 함수 분해

3.3.1 규칙: 호출 또는 전달, 한가지만 할 것

정의

함수 내에서는 객체에 있는 메서드를 호출하거나 객체를 인자로 전달할 수 있지만 둘을 섞어 사용해서는 안된다.

설명

  • 더 많은 메서드를 도입하고 여러 가지를 매개변수로 전달하기 시작하면 결국 책임이 고르지 않게 될 수 있다.
  • 함수에서 배열에 인덱스를 설정하는 것과 같은 직접적인 작업을 수행하거나 동일한 배열을 더 복잡한 함수에 인자로 전달할 수도 있다. 그러면 코드는 직접 조작하는 낮은 수준의 작업과 다른 함수에 인자로 전달하는 높은 수준의 호출이 공존해서 메서드 이름 사이의 불일치로 가독성이 떨어질 수 있다.
  • 동일한 수준의 추상화를 유지하는 편이 코드를 읽기가 쉽다.

배열의 평균을 구하는 함수를 생각해자. 이것은 높은 수준의 추상화 sum(arr)과 낮은 수준의 arr.length를 모두 사용한다.

// 배열의 평균을 구하는 함수 (변경 전)
function average (arr:number[]) {
  return sum(arr). arr.length;
}

// 변경후
function average(arr:number[]){
  return sum(arr) / size(arr);
}

스멜

'함수의 내용은 동일한 추상화 수준에 있어야 한다' 는 말은 그 자체가 스멜일 정도로 강력하다.
전달된 인자의 메서드가 어떻게 사용되었는지를 식별하는 것은 간단한 일인데, 인자로 전달된 변수 옆의 .(점)으로 쉽게 찾을 수 있다.

의도

메서드에서 몇 가지 세부적인 부분을 추출해서 추상화를 도입할 떄 이 규칙은 연관된 다른 세부적인 부분도 추출하게 한다. 이렇게 하면 메서드 내부의 추상화 수준이 항상 동일하게 유지된다.

3.3.2 규칙 적용

draw 메서드를 살펴보면 이 규칙을 위반했음을 바로 알 수 있다. 변수 g는 매개변수로 전달되기도 하고 거기에 메서드를 호출하기도 한다.

function draw() {
  let canvas = document.getElementById("GameCanvas") as HTMLCanvasElement;
  let b = canvas.getContext("2d");
  
  g.clearRect(0, 0, canvas.width, canvas.height);
  
  drawMap(g);
  drawPlayer(g);
}

메서드 추출을 사용해서 이 규칙 위반을 수정해보자.
코드에서 g.clearRect줄을 추출하면 결국 canvas를 인지로 전달하면서 canvas.getContext를 호출하게 돼서 다시 규칙을 위반한다.
대신, 처음 세 줄을 함께 추출해보자. 메서드 추출을 수행할 떄마다 메서드 이름을 지어 코드를 더 알기 쉽게 만들 수 있다.

3.4 좋은 함수 이름의 속성

좋은 이름이 가져야 할 몇 가지 속성이 있다.

  • 정직해야한다. 함수의 의도를 설명해야 한다.
  • 완전 해야한다. 함수가 하는 모든 것을 담아야한다.
  • 도메인에서 일하는 사람이 이해할 수 있어야한다. 작업 중인 도메인에서 사용하는 단어를 사용하라. 그렇게 하면 의사 소통이 더욱 효율적이게 되고 팀우너 및 고객과 코드에 대해 더 쉽게 이야기 할 수 있다는 장점이 있다.
function draw() {
  let canvas = document.getElementById("GameCanvas") as HTMLCanvasElement; // 무언가를 그릴 html 엘리먼트를 가져옴
  let b = canvas.getContext("2d"); // 캔버스를 인스턴스화
  
  g.clearRect(0, 0, canvas.width, canvas.height); // 캔버스를 지움
  
  ...
}

위의 세줄은 간단히 말해서 그래픽 객체를 만든다.

let g =createGraphics();
drawMap(g);
drawPlayer(g);

update로 넘어가기!

function update() {
  while (inputs.length > 0) {
    let current = inputs.pop();
    if (current === Input.LEFT)
      moveHorizontal(-1);
    else if (current === Input.RIGHT)
      moveHorizontal(1);
    else if (current === Input.UP)
      moveVertical(-1);
    else if (current === Input.DOWN)
      moveVertical(1);
  }

  for (let y = map.length - 1; y >= 0; y--) {
    for (let x = 0; x < map[y].length; x++) {
      if ((map[y][x] === Tile.STONE || map[y][x] === Tile.FALLING_STONE)
        && map[y + 1][x] === Tile.AIR) {
        map[y + 1][x] = Tile.FALLING_STONE;
        map[y][x] = Tile.AIR;
      } else if ((map[y][x] === Tile.BOX || map[y][x] === Tile.FALLING_BOX)
        && map[y + 1][x] === Tile.AIR) {
        map[y + 1][x] = Tile.FALLING_BOX;
        map[y][x] = Tile.AIR;
      } else if (map[y][x] === Tile.FALLING_STONE) {
        map[y][x] = Tile.STONE;
      } else if (map[y][x] === Tile.FALLING_BOX) {
        map[y][x] = Tile.BOX;
      }
    }
  }
}

이 코드를 자연스럽게 두 개의 더 작은 함수로 나눌 수 있다.
첫 번쨰 그룹에서 지배적인 단어가 input이고, 두 번째 그룹에서는 지배적인 단어가 map 이라는 것을 알 수 있다. update라는 함수를 분할하고 있으므로 첫 번째 안으로 updateInputs, updateMap 이라는 함수명을 얻을 수 있다. 그러나 아마도 입력 값을 수정하지는 않을것이다. 따라서 다른 명명 방식으로 handle 이라는 용어를 사용한다. handleInputs

3.5 너무 많은 일을 하는 함수 분리하기

function update() {
  handleInputs();
  updateMap();
}
function handleInputs(){
   while (inputs.length > 0) {
    let current = inputs.pop();
    if (current === Input.LEFT)
      moveHorizontal(-1);
    else if (current === Input.RIGHT)
      moveHorizontal(1);
    else if (current === Input.UP)
      moveVertical(-1);
    else if (current === Input.DOWN)
      moveVertical(1);
  }
}

function updateMap() {
  for (let y = map.length - 1; y >= 0; y--) {
    for (let x = 0; x < map[y].length; x++) {
      if ((map[y][x] === Tile.STONE || map[y][x] === Tile.FALLING_STONE)
        && map[y + 1][x] === Tile.AIR) {
        map[y + 1][x] = Tile.FALLING_STONE;
        map[y][x] = Tile.AIR;
      } else if ((map[y][x] === Tile.BOX || map[y][x] === Tile.FALLING_BOX)
        && map[y + 1][x] === Tile.AIR) {
        map[y + 1][x] = Tile.FALLING_BOX;
        map[y][x] = Tile.AIR;
      } else if (map[y][x] === Tile.FALLING_STONE) {
        map[y][x] = Tile.STONE;
      } else if (map[y][x] === Tile.FALLING_BOX) {
        map[y][x] = Tile.BOX;
      }
    }
  }
}

3.5 너무 많은 일을 하는 함수 분리하기

3.5.1 규칙: if 문은 함수의 시작에만 배치

정의

if 문이 있는 경우 해당 if 문은 함수의 첫 번째 항목이어야 한다.

설명

  • 함수는 한 가지 일만 해야하며 무언가를 확인하는 것은 한 가지 일이다.
  • 함수에 if가 있는 경우 함수의 첫번째 항목이어야 한다. 또한 그 후에 아무것도 해서는 안된다는 의미에서 유일한 것이어야한다.
  • else문과 분리해서는 안된다. 본문와 else는 모두 코드 구조의 일부이며 이 구조에 의존해서 작업하므로 코드를 이해할 필요가 없다. 동작과 구조는 밀접하게 연결되어 있으며 리팩터링할 때 동작을 변경해서는 안되므로 구조도 변경해서는 안된다.

예제

2에서 n까지의 수에서 소수를 출력하는 함수

function reportPrimes(n:number) {
  for(let i = 2; i < n; i++)
    if(isPrime(i))
      console.log(`${i} is Prime`);
}

적어도 두 가지 분명한 작업이 존재한다.

  • 숫자를 반복한다.
  • 숫자가 소수인지 확인한다.

따라서 최소한 두 개의 함수가 있어야한다.

// 변경 전
function reportPrimes(n:number) {
  for(let i = 2; i < n; i++)
    if(isPrime(i))
      console.log(`${i} is Prime`);
}

// 변경 후
function reportPrimes(n:number){
   for(let i = 2; i < n; i++)
  	reportIfPrime(i);
}

function reportIfPrime(n:number){
  if(isPrime(n))
    console.log(`${n} is prime`);
}

무언가를 확인하는 것은 하나의 작업이며, 하나의 함수에서 처리해야한다. 그래서 이 규칙이 필요하다~!

스멜

다섯 줄 제한과 같이, 이 규칙은 함수가 한 가지 이상의 작업을 수행하는 스멜을 막기 위해 존재한다.

의도

이 규칙은 if 문이 하나의 작업이기 때문에 이를 분리할 때 이어지는 else if 는 if 문과 분리할 수 없는 원자 단위로 본다.

3.5.2 규칙 적용

추출하려는 함수명을 정하려면 추출하려는 코드를 표면적으로 봐야한다. 이 그룹의 줄에는 map과 title이라는 단어가 두드러져 보인다. 이미 updateMap이라는 함수를 가지고 있으므로 새로운 함수를 updateTitle 이라 한다.

function update() {
  handleInputs();
  updateMap();
}
function handleInputs(){
   while (inputs.length > 0) {
    let current = inputs.pops();
     handleInput(current);
}

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

function updateMap() {
  for (let y = map.length - 1; y >= 0; y--) {
    for (let x = 0; x < map[y].length; x++) {
      updateTitle(x,y);
    }
  }
}

function updateTitle(x:number, y:number) {
  if ((map[y][x] === Tile.STONE || map[y][x] === Tile.FALLING_STONE)
        && map[y + 1][x] === Tile.AIR) {
        map[y + 1][x] = Tile.FALLING_STONE;
        map[y][x] = Tile.AIR;
      } else if ((map[y][x] === Tile.BOX || map[y][x] === Tile.FALLING_BOX)
        && map[y + 1][x] === Tile.AIR) {
        map[y + 1][x] = Tile.FALLING_BOX;
        map[y][x] = Tile.AIR;
      } else if (map[y][x] === Tile.FALLING_STONE) {
        map[y][x] = Tile.STONE;
      } else if (map[y][x] === Tile.FALLING_BOX) {
        map[y][x] = Tile.BOX;
      }
}

updateMap, handleInputs 메서드 추출 작업을 끝냈다. 여기서 메서드 추출의 또 다른 장점을 볼 수 있다.

  • 새로운 메서드에서 더 많은 정보를 제공할 수 있는 새로운 이름을 매개변수에 지정해 가독성을 높인다. current는 루프 변수에 좋은 이름이었지만 새로운 handleInput 함수에서는 input이 잘어울린다.

요약

  • 다섯 줄 제한 규칙은 메서드는 다섯 줄 이하여야 한다는 말이다. 이것은 두 가지 이상의 작업을 수행하는 메서드를 식별하는데 도움이 된다. 리팩터링 패턴인 메서드 추출을 사용해서 긴 메서드를 분해하고 메서드 이름으로 주석을 대신한다.

  • 호출 또는 전달, 한 가지만 할 것 규칙은 하나의 메서드 내에서 객체에 있는 메서드를 호출하거나 객체를 매개변수로 전달할 수 있지만, 둘 다 해서는 안 된다는 말이다. 여러 수준의 추상화가 섞여 있는 메서드를 식별하는 데 도움이 된다. 이 또한 메서드 추출을 사용해 서로 다른 수준의 추상화를 분리한다.

  • 메서드 이름은 투명하고 완전해야 하며 이해할 수 있어야 한다. 메서드 추출을 사용하면 가독성이 향상되도록 매개변수의 이름을 바꿀 수 있다.

  • if 문은 함수의 시작에만 배치 규칙은 if를 사용해 조건을 확인하는 경우 한 가지 작업만 수행하므로 메서드가 다른 작업을 수행하지 못하게 한다. 이 규칙은 또한 하나 이상의 작업을 수행하는 메서드를 식별하는 데 도움이 된다. if 문을 분리하기 위해 메서드 추출을 사용한다.

profile
공효은
잼나게 코딩하면서 살고 싶어요 ^O^/
이전 포스트

[리팩터링] 2장 리팩터링 깊게 들여다 보기

다음 포스트

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

0개의 댓글