[코드리뷰] 자동차 경주 게임

1. 명시적인 이름을 사용하자.

각 함수들이 어떤 역할을 하는지 조금 더 명시적인 이름을 사용한다면 어떤 일을 하는지 더 알기 쉬울 것 같다고 알려주셨다.

어디서 주워들은게 많아서^^... set-*, get-*, init-* 등의 네이밍을 생각없이 남발했었는데, 이런 네이밍이 언제 쓰여야 하는지 조금 알게된 것 같다. 알게된 내용을 짧게 아래에 정리해보겠다!

1-1. set-* 네이밍

setXXX이라는 이름의 함수는 관례적으로 setting할 인자를 받는 식으로 구성이 된다고 한다. 예를 들자면 👉👉setXXX(setting할 인자) 이렇게! 그래서 만약 setSectionDataId()라고 이름을 정했다면, setSectionDataId(id)와 같은 형식으로 id를 넣어주는 식으로 구성을 하는 것이 더 맞는 네이밍이라고 한다. 나는 setSectionDataID() 함수의 내용에 각 <section>마다 data-id를 부여해주는 코드를 작성했는데, 이런 경우처럼 특정 데이터를 초기화하는 로직에 set 네이밍은 맞지 않다고 하셨다. 초기화하는 로직에는 init이 더 적합하다고 하셨다.

1-2. get-* 네이밍

getXXX이라는 이름의 함수는 관례적으로 XXX를 리턴하도록 구현한다고 한다. 그래서 만약 getCarInstance()라는 네이밍을 주었다면, const car = getCarInstance()와 같은 식으로 코드가 짜여야 한다고 했다. 하지만 내가 짠 getCarInstance() 함수의 내용은 state.car에 Car를 할당하는 내용이기 때문에 이와 맞지 않았다.  

 

1-3. isValid-* 네이밍

isValid는 valid할 때 true를 반환하고, invalid할 때에는 false를 반환해야한다. 코드를 짜다가 네이밍과 반대로 리턴되게 함수를 만들어 버렸다🥺🥺. 사소하지만 네이밍에 신경써서 리턴값이 그에 알맞게 반환되도록 유의해야겠다.

 

1-4. 기타 네이밍

resetView([2,3,4])와 같은 함수에서 인자로 [2,3,4] Array를 넘겨주었었는데, 사실 이 코드만 보면 어떤 view를 reset하는지 보기 어려웠다. 이런 경우에도 단순 숫자를 넣는 것이 아니라, 숫자에 이름을 붙여 무엇을 reset하는지 구체적으로 알 수 있게 해야한다는 것을 다시 깨달았다. 

showCarName() 함수 또한 차 이름을 보여주는 역할을 하는 것이 아니라, 차 이름이 들어간 div DOM 객체를 반환하는 코드였다. DOM 객체를 반환하는 것과 show라는 이름이 맞지 않아 의도에 맞게 함수의 이름을 carNameComponent()로 바꾸는 것이 더 나은 코드라고 말씀주셨다.

 

이름을 짓는 것은 협업을 함에 있어서 굉장히 중요한 요소 중 하나라고 생각한다. 이번 미션에서 이런 네이밍 문제가 꽤 많았는데😂😂, 나만 보기 쉬운 코드가 아니라, 모두가 읽기 좋고 보기 쉬운 코드를 만들 수 있도록 노력해야겠다는 생각이 절실히 들었다.

2. for문을 지양하고 Array의 method 들을 적극적으로 사용하자.

for (let idx=0; idx < sections.length; idx++) {}

위와 같은 for문은 반복의 idx를 카운팅할 변수와 조건, 그리고 인덱스를 하나씩 올리는 것까지 다양한 요소들에서 인간이 실수할 곳이 많기 때문에 실무에서도 잘 사용하지 않는다고 하셨다. for문 대신 Array의 method들을 잘 사용한다면 코드가 더 깔끔하고 유지보수 측면에서도 더 나아질 것이라고 말씀주셨다. 그리고 이런 method들을 잘 연결해 사용하면(체이닝) 훨씬 코드가 간결해 질 수 있다는 것을 느꼈다. 

export const getWinner = () => {
  .....

  return winners.map((winner) => {
    return winner.name;
  });
};

위와 같이 작성했던 코드를 아래와 같이 작성하면 한 줄에 작성할 수 있었다. 하지만 나는 아직 Array의 메소드들에 대해서도 익숙하지 않아 state.cars.filter((car) => car.totalStep === maxTotalStep) 을 하나의 변수에 담아주고, 그 변수에서 다시 map(car => car.name)을 사용해 값을 리턴해 주는 방식을 택했다.

return state.cars.filter((car) => car.totalStep === maxTotalStep).map(car => car.name);

 

안그래도 페어와 코딩하면서 언제 forEach를 사용하고 map을 사용하는지에 대해 이야기를 나눈 적이 있다. 그 당시에도 내가 Array의 method들을 잘 사용하지 못하고 있다는 것을 깨달았었는데 이번 코드리뷰를 받으며 새삼 다시 깨닫게 되었다. 하나씩 차근차근 배워나가야겠다!

forEach와 map의 다른 점에 대해 간단히 정리하고 넘어가자면, forEach는 return이 없을 때 사용하고, map은 return이 있을 때 사용한다고 한다. 내가 위에 만든 코드는 setSectionDataID() 함수로, 위에 언급했듯이 <section>에 data-id 속성을 넣어주는 역할을 했는데, 이는 return이 없기 때문에 forEach를 사용하는 것이 더 적합한 것이었다. 

3. 함수 내에서 return을 하지 않아도, 명시적으로 false를 return해주자.

isXXX 라는 네이밍의 함수를 만들어 사용하면서, boolean 값을 리턴하도록 했는데, false를 리턴하지 않았었다. javascript는 함수 내에서 return을 하지 않아도 함수를 종료한다면 암시적으로 undefined를 리턴하기 때문에 falsy하게 처리가 되기는 하지만, 명시적으로 false를 리턴해주는 것이 좋다고 하셨다.

4. 한 개의 함수 내에서는 한 개의 역할만 하도록 하자.

validation 함수에 return true/false를 하게 만들면서 중간에 alert도 넣어주고, input을 없애주는 역할도 만들어서 넣어버렸다. 내가 만든 isCarNameEmpty 함수는 carName이 Empty한 지 아닌지를 알려주는 책임만 one and only하게 가져야 하고, 나머지 로직들은 밖으로 빼서 사용하는 것이 좋다고 하셨다. 그래야 재사용도 용이하고, 불필요한 side effects들을 줄일 수 있는 방향이라고 알려주셨다. 함수는 하나의 역할만 하게 만드려고 했는데, 코드를 의식의 흐름대로 작성하다보니 생긴 문제 같았다. 다음부터는 어떤 함수를 만들어야 할 것인지에 대해 조금 더 명확히 하고 코드를 작성한다면 이런 문제들을 줄일 수 있을 것 같다. 

5. DOM을 조작할 때에는 id나 data속성을 이용하자.

이번 자동차 게임 미션을 하면서 data-속성을 잘 활용하지 못해 section들을 거의 getElementsByTagName으로 가져왔는데, 이렇게 코드를 짜두면 나중에 html이 변경되는 등의 이벤트가 발생했을 때, 코드가 망가질 수 있어 이렇게 DOM을 조작하는 것은 지양하라고 하셨다. 이 내용에 있어서는 어느정도 인지를 하고 있었음에도 불구하고 충분히 고민을 하지 않고 개발에 들어갔던 것이 문제였던 것 같다. id로 값을 가져오는 것은 어느정도 잘 다룰 수 있으니, 다음부터는 data 속성을 부여하고 그를 통해 코드를 짜는 방향을 연습해봐야겠다.

6. 간결한 코드를 만들자(feat. 삼항연산자)

프리코스 리뷰에서, 그리고 여기저기 전해들은 이야기들을 통해 삼항연산자는 가독성이 떨어지기 때문에 지양하는 것이 좋다고 봐서 삼항연산자에 대해 따로 열심히 알려고 노력하지 않았다. 삼항연산자로 작성된 내용을 읽을 수는 있지만, 내가 삼항연산자를 사용해 코드를 직접 작성하기는 힘든? 그런 상태였다. 그런데 코드 리뷰에서 삼항연산자를 보게 되었다! 뚜둔! 🤭🤭

먼저 정말 신기했던 건, 내가 약 8줄의 코드로 작성했던 내용이, 단 한줄로 줄여질 수 있다는 것이었다. 리뷰어분께서 써주신 내용에 뭔가 머리를 한대 얻어맞은 것 같았다 ^^,, 내가 간과하고 있던 부분에 대해 뼈를 때려주셨다. 호홋. 아래는 리뷰어분께서 써주신 원문이다.

const getWinnerText = (winners) => winners.length === 1 ? winners[0] : winners.join(", ")
// 처럼 삼항연산자를 이용하시면 좀 더 간결하게 작성 하실 수 있습니다.
// (때로는 삼항연산자의 사용이 가독성을 더 떨어뜨릴 수 있으니 트레이드오프로 이점은 적절히 선택 하시면 되겠습니다.)
// 코드가 짧다고 무조건 좋은것은 아니지만 일반적으로는 더 좋은 경우가 많습니다.
// 따라서 연습중에는 가능하면 더 짧게 작성하려고 해보시고 가독성이 너무 떨어지는것 같으면 적절하게 변수처리도 하고 식을 문으로 바꿔보기도 하면서 다양한 방법으로 시도해보시면 좋습니다.

삼항연산자의 장점과 단점에 대해 알려주셨고, 이에 따라 상황에 맞게 적절히 사용하는 것이 좋다고 하셨는데, 지금까지 "아 안좋으니까 쓰지말아야지~ 굳이 더 알아야하나?" 라고 생각했던 내 자신이 부끄러워지는 순간이었다. 말씀주신대로 연습 중에는 다양한 시도를 통해 코드를 작성해보고, 어떤 방식이 좋을지 스스로 찾아내보는 시간을 가져야겠다고 생각했다. 단순히 삼항연산자를 적절히 사용해서 쓰자!를 배운 것이 아니라 코드를 작성하는 나의 태도에 대해서도 다시 한 번 돌아볼 수 있어서 이 리뷰가 정말 나에게 와닿았다. 앞으로의 연습에서는 겸손한 마음으로 개발에 임해야겠다는 생각이 들었다. 

7. 객체의 prototype을 잘 사용해보자(feat. 메모리 효율)

function Car(name) {
  this.name = name;
  this.totalStep = 0;

  this.go = () => {
  	this.totalStep += 1;
  };
}

아직 class와 prototype에 익숙하지 않아, 객체를 만들 때마다 각 인스턴스 별 메소드를 생성했다. 그런데 사실 이 go()라는 메소드는 굳이 Car 객체를 만들 때마다 선언해 줄 필요가 없었다. 이렇게 되면 인스턴스 별로 go() 메소드가 만들어지기 때문에 메모리 효율 면에서 좋지 않다는 것을 알게 되었다. (new Car 을 할 때마다 Car 안에 go라는 메소드가 생성되기 때문에!) 그래서 굳이 이렇게 만들지 않고, 아래처럼 따로 prototype을 이용해 코드를 작성하는 게 더 메모리 효율적으로 좋다고 하셨다. 그러면 new Car을 할 때마다 go라는 메소드를 생성하지 않고도 사용할 수 있다! (신기) 

프로토타입은 메모리 복사가 아닌, __proto__라는 참조 변수를 통해 연결되므로 한 메모리 공간을 차지하기 때문에 함께 사용하는 식별자가 존재할 때에는 프로토타입 체인을 연결함으로써 메모리 효율을 높일 수 있는 것이다.

Car.prototype.go = function() {
  this.totalStep += 1;
}

흠.. Class와 Prototype에 대해 조금 더 공부를 해야겠다. 아직 코드를 구현하는 데 급급해 메모리 효율성에 대해서도 크게 고민해 본 적이 없는데, 공부하면서 같이 생각해봐야겠다.!!

 

같은 맥락에서, 이번 자동차 경주 미션을 수행하면서 매번 Section을 hide하고 show했어야 했는데, 매번 section을 DOM으로 가져오고 element.style.display = "block" , element.style.display = "none" 을 따로 함수로 지정해 사용했다. 그런데 리뷰어분께서 이렇게 작성했던 코드를 따로 Section class로 만들어 show와 hide를 내부의 메소드로 만들어서 사용하는 방법을 알려주셨다! 뭔가 새로운 세계에 눈을 뜬 느낌이다 ^^;; 위에 공부했던 메모리 효율성도 고려해서 prototype을 이용해 메소드를 밖으로 빼주었다. 

수정!! 

클래스로 만들 때에는 아래와 같이 만들어 줄 필요가 없다..... 그냥 내부 메소드로 선언하면 된다. 궁금해서 아래와 같이 직접 돌려봤는데 똑같은 것 같다. 근데 Class에서는 굳이 저렇게 작성해 줄 필요 없다!!

아래는 안좋은 예시!

export default class Section {
  constructor(element) {
    this.element = element;
  }
}

Section.prototype.show = function () {
  this.element.style.display = "block";
};

Section.prototype.hide = function () {
  this.element.style.display = "none";
};

아래 블로그가 프로토타입에 대해 잘 써놓은 것 같다.

medium.com/@bluesh55/javascript-prototype-%EC%9D%B4%ED%95%B4%ED%95%98%EA%B8%B0-f8e67c286b67

[Javascript ] 프로토타입 이해하기

 

Section을 class로 만들면서 구조 분해 할당에 대해서도 배우게 되었는데, 기존에 매번 DOM으로 가져오던 내용들을 아래와 같이 쉽게 가져와 변수에 할당할 수 있었다. 구조 분해 할당과 같은 경우에도 삼항연산자와 같이 개념에 대해서는 막연하게 알고 있었지만, 활용을 잘 못했었는데 리뷰를 통해 어떻게 사용해야 할 지 조금 감이 잡혔다. 여기에서도 Array의 메소드를 잘 사용할 수 있어야겠다는 생각을 했다.

구조 분해 할당에 대해서는 리뷰어분께서 첨부해주신 링크를 보고 조금 더 가닥을 잡을 수 있었다. 하지만 아직도 어렵다 😔😔.. 잘 활용할 수 있도록 연습에서 많이 사용해봐야겠다.

https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment

구조 분해 할당 - JavaScript | MDN

export const [gameTitleSection, carNameSection, countSection, carPlayerSection, resultSection ] = Array.from(document.getElementsByTagName("section")).map(it => new Element(it))