Lv1 자동차 경주 미션 피드백 1 : [우아한테크코스 5기 AN_베르]

현직자 리뷰어 님의 피드백을 바탕으로 정리하였습니다.

 

페어 프로그래밍 리뷰

 페어 프로그래밍을 처음해봐서 아직 제대로 이해하지 못한 코드를 많이 적용하게 되었고 그러다 보니 이해도가 점점 떨어지게 되었다. 리뷰 또한, 당연하게도 제대로 이해하지 못한 것들의 질문을 많이 받게 되었다. 그 질문들에 대한 답을 간단하게 정리해 본다.

 

 

1. 스켈레톤 코드 커밋

  프리코스 때 부터 시작하기 전에 대략적인 구조를 만들고 그 구조에 맞게 클래스를 미리 정의하고 커밋했다. 기능을 구현하는 과정에서 어떻게 구현을 해 나갈지 모르고 구현 시 코드에 많은 변경이 있을 것이다. 따라서 작성자 본인 외에 알기 어려운 커밋 단위는 좋지 않은 커밋이다.  귀찮음을 피하는 것은 커밋으로 해결해선 안된다.

 

2. 멤버 변수로 사용할 필요가 있는가?

- 기존 코드

class Controller(...){
    private val cars = mutableListOf<Car>()
    private val judgement = Judgement(cars)
    private var time = 0
    ...
    }

• 장점
  사용할 주요 객체를 미리 명시할 수 있고 같은 클래스 내에서 인자로 넘겨주지 않아도 되는 편의성을 제공합니다.
• 단점
  클래스가 인스턴스화되면 초기화되어 불필요하게 메모리를 차지하고 있습니다. 만약 같은 게임을 여러번 하면 재사용이 힘들어 전체 구조를 바꿔야할 수 있습니다.
• 결론
  개발자에게 편의성을 제공하는게 효율적인 코드는 아니기 때문에 멤버 변수로 만들지 않고 필요할 때 생성하는게 좋을 것 같습니다.

 

- 변경한 코드

fun run(){
	val cars = createCars()
	val raceTime = getRaceTime()
}

변수가 필요한 범위를 가장 좁은 곳 부터 확장해 나가는 방법이 좋다. 정말 멤버 변수로 필요한지 생각해보아야 한다.

 

3. Map 의 올바른 사용 방법

- 기존 코드

cars.map { car ->
            outputView.printRaceResult(car.race())
        }

  위의 코드 또한 전체적인 흐름에서 정상적으로 동작한다. 다만 맵은 정의한 리스트를 만들어 반환하는 용도로 사용된다. 위 코드는 반환없이 실행만 필요하기 때문에 stream 함수 중에 forEach 로 변경하여 용도에 맞게 사용해야 한다.

 

4. 클래스, 함수, 변수의 이름을 용도에 맞게 지어라

  굉장히 많은 코멘트를 받은 피드백이다. 각 이름은 어떤 역할을 수행하는지 혹은 값을 나타내는지 명확히 한눈에 알 수 있도록 지어야 한다. 또한 클래스나 함수에서 사용되는 이름이 상대적이어선 안된다. 예를 들어 상위에 우승한 차 이름을 찾는 기능을 구현할 때 Car 클래스 내부에서 이름을 가져와야 하면 findWinnersName() 으로 정의하면 안된다. Car의 입장에서는 이름을 반환하는 것이지 우승자를 찾는 것인지 몰라야 하기 때문이다. 따라서 getName() 등으로 이름을 반환하는 것이 올바르다. 물론 kotlin 에서는 var로 선언 시 getter와 setter를 자동으로 만들어 주기 때문에 완전히 적절한 예시는 아니다.

 

 

5. DTO 란 무엇일까?

- DTO 의 용도

   계층간의 의존성 없이 데이터를 전달하기 위해 사용한다.  

 

  페어 프로그래밍 할 때 코드에 DTO 파일을 많이 만들어 사용했다. 하지만 DTO 내부에서 검증을 하거나 값을 정제하는 로직을 갖게 만들었고 이는 본래의 용도를 이해하지 못하고 사용한 결과였다. DTO를 정확히 알고 사용하라는 리뷰에 따라 아래에 DTO 를 간단하게 정리해 본다.

 

DTO 가 의존성 없이 데이터를 전달해야 하므로 단순히 데이터를 전달하기 위한 바구니로 사용돼야 하지 그 외의 용도로는 사용되면 안된다.

 

  접근 당하는 클래스가 많은 프로퍼티를 가진다면 클래스는 private, public 등 접근 제한자를 사용하여 캡슐화가 가능하다.

단, 접근하려는 클래스가 여러 개고 각기 다른 프로퍼티에 접근한다면 접근 당하는 클래스는 모든 프로퍼티를 public 으로 해야할 수 있다.

이 떄 DTO를 사용한다면 다음과 같은 장점이 있다.

 

- 필요한 정보만 주고 나머지 정보는 은닉할 수 있다.
- 정보를 주더라도 본래의 값을 변경하지 못하게 하기 때문에 안전하다.

 

  하지만 여기서 DTO를 이용해 필요한 정보만 준다는 것은, 필요한 곳과 강한 커플링이 된다는 뜻이 될 수도 있다.
가령 UI 모델에서 사용하기 위한 DTO인데 UI가 변경되면 DTO를 비롯해 도메인 로직이 같이 수정이 되야 할 수도 있다.

 

이 부분에 대해서는 더 깊이 공부해 봐야 할 것 같다.

 

DTO의 용도를 고민해 보면서 이번 미션에서 DTO의 필요성을 느끼지 못했고 또한 제대로 알지 못하는 것은 사용하지 말아야겠다 생각이 들어서 삭제하였다.

 

6. 테스트 코드를 명확하게 나타내라

// 기존
@Test
fun `자동차 이동 테스트` () {}

// 변경
@Test
fun `자동차가 4 이상의 값이면 한 칸 앞으로 전진한다` () {}

테스트 코드는 프로그램의 명세서다. 즉 테스트 이름으로 하여금 어떤 동작을 하는지 예측이 가능해야 한다. 무엇을 하는 기능인지, 성공하면 성공 이유를, 실패하면 실패 이유를 기능 단위로 명확하게 나타내야 한다.

 

// 1차
fun `자동차 공장에 문제가 없다면 오류가 발생하지 않는다`()

// 2차
fun `자동차 개수가 1 이상 20대 이하이고 중복이 없다면 에러가 발생하지 않는다`()

// 3차
fun `자동차는 2대에서 20대 이하여야 하고 이름에 중복이 없어야 한다`()

 

- 1차의 문제점 : 매우 추상적이며 문제가 없다면 어떤 문제가 없는지 오류라는 것은 어떤 오류인지 명확하지 않다.

- 2차의 문제점 : 에러가 발생하지 않는다는 말은 보는 사람으로 하여금 다르게 해석될 여지가 있다.

 

정리 및 소감

1. 잘 모르면 사용하지 마라. (전에 선배 개발자에게 들었던 얘기인데 다시 한 번 상기하게 되었다)

2. 쓸 필요가 없으면 사용하지 마라.

3. 뭐든지 명확히 하라.