효과적인 코드리뷰를 위한 리뷰어 자세

👉🏻 카카오 테크
https://tech.kakao.com/2022/03/17/2022-newkrew-onboarding-codereview/

• 리뷰어 : 리뷰 하는 사람
• 리뷰이 : 리뷰 당하는 사람

코드리뷰

한 개발자가 코드를 작성하면 다른 개발자가 정해진 방법으로 피드백 주고 받는 과정

• 본인이 발견하지 못한 실수를 발견하여 코드의 부작용과 오류 조기 대응
• 정해진 컨벤션 규칙 유지, 기술 부채 감소

리뷰의 5가지 원칙

1. 왜 개선이 필요한지 충분히 설명해주기

const data = [
  ['데이터베이스', 'A', 3],
  ['교양영어', 'B+', 1],
  ['철학', 'A', 2]
];

• 안 좋은 리뷰
  “data 변수 말고 다른 변수명으로 하세요."
  “data 변수 말고 grade로 하세요.”

• 좋은 리뷰
  “data라는 이름은 현재의 자료구조가 무엇인지 그 의도를 알기가 어렵네요.
  학점 정보를 담고 있는 자료구조 같은데 이와 관련된 변수명을 짓는다면
  현재 정의한 자료구조가 무엇인지 그 의도를 쉽게 파악할 수 있을 것 같아요.”

2. 답을 알려주기보다는 스스로 고민하고 개선 방법을 선택할 수 있게 해주기

const result = [];
arr.forEach(item => {
  if(item % 2 === 0) {
    result.push(item);
  }
});

• 안 좋은 리뷰
  “arr.filter(item => item % 2 === 0);으로 사용하세요."

• 좋은 리뷰
  “자바스크립트에는 배열에는 다양한 내장 메서드들이 존재합니다.
  그중 filter 메서드를 활용해 보세요.
  이 메서드를 활용하면 코드량을 줄일 수 있습니다.”

3. 코드를 클린 하게 유지하고, 일관되게 구현하도록 안내해주기

const checkNumber = (comNum, myNum) => {
  return myNum.reduce((prev, curr) => {
    if (comNum[curr]) return prev + 1;
    return prev;
  }, 0);
}
function calculateEarningRate(list, totalPrizeMoney, investMoney) {
  const result = [];
  for(let i=0; i < list.length; i += 1) {
    result.push((list[i] / investMoney) * 100);
  }
  return result;
}

• 안 좋은 리뷰
  "함수 표현식을 사용하셨군요. reduce의 사용도 잘 활용했네요."
  => 전체 코드를 본게 아니라 부분적으로 보고 판단한 경우

• 좋은 리뷰
  "함수를 선언하는 두 가지 방식(함수 표현식, 함수 선언식)을 사용했는데
  특별한 이유가 아니라면 함수를 선언하는 방식을 스스로 정하고 그 컨벤션 규칙을
  따르도록 해보세요. 일관되게 동작하는 코드는 훨씬 보기 좋고 쉽게 수정할 수 있습니다.
  그리고 reduce를 통해서 새로운 자료구조를 만든 건 잘했습니다.
  하지만 같은 반복처리를 하는 과정에서 calculateEarningRate 에서
  for문을 사용한 건 아쉽네요. reduce와 비슷한 방식의 다른 고차 함수를 찾아서
  써보면 더 일관된 코드를 유지할 수 있을 거 같습니다."

4. 리뷰 과정이 숙제 검사가 아닌 학습과정으로 느낄 수 있게 리뷰해주기

class Component {
    constructor(node) {
        this.node = node;
    }

    render(child) {
        this.node.appendChild(child);
    }
}

class Header extends Component {
    constructor(node) {
        super(node);
    }
}

class List extends Component {
    constructor(node) {
        super(node);
    }
}

• 안 좋은 리뷰
  “클래스 상속은 필요 없습니다.”
  => 리뷰이의 의도는 파악하지 않은 채 클래스 상속은 필요 없다고 단정. 의도 물어보는게 중요.

• 좋은 리뷰
  "Component 클래스를 상속 받는 건 좋네요.
  그러나 자식 클래스에서 부모 클래스를 호출만 하기 때문에 모든 클래스에서
  상속받는 건 오히려 중복 코드 같아 보이기도 하는데 이렇게 작성해 주신 이유가 있을까요?"
  => 먼저 ‘중복 코드’라는 단어를 사용하여 상속의 불필요한 이유를 설명하고 있습니다. 그리고 리뷰이의 생각은 다를 수 있으므로 의도를 물어봅니다.

5. 피드백 할 게 없으면 칭찬해 주세요.

잘 짜여진 코드를 억지로 리뷰할려다가 잘못된 방향으로 이끌 수 있다. 피드백 할게 보이지 않는다면 칭찬을 하자.

• 칭찬 예시
  코드량이 적당해서 읽기 편하네요.
  많은 고민이 코드에서 엿보이네요.
  성능에 아주 유리한 코드라고 생각되네요.
  전에 코드보다 훨씬 좋아진 거 같네요.
  예외 처리가 꽤 꼼꼼해서 좋네요.
  함수, 변수명이 직관적이어서 좋네요.