티스토리 뷰

728x90

 

 

미션 관련 링크

로또 레포지토리

https://github.com/kimhm0728/kotlin-lotto
로또(자동) 1단계 PR

https://github.com/woowacourse/kotlin-lotto/pull/74

로또 (수동) 2단계PR
https://github.com/woowacourse/kotlin-lotto/pull/94

 

 

 

코드 리뷰 받은 코멘트들 

상수를 관련 클래스에 포함시키자

기존에는 상수만을 갖는 클래스를 생성했다. (LottoConstants)

여러번 사용되는 상수는, 해당 상수를 사용하는 클래스에 각각 선언하는 것은 중복 코드라고 생각했기 때문이다.

또한 하나의 클래스에 상수를 선언하고, 다른 클래스에서는 그 클래스의 상수를 참조한다면, "상수를 선언하는 클래스를 어떻게 정할지"도 고민이 되었다.

그런데 클래스가 수백개 수천개가 된다면, 상수가 2번 이상 사용되는지 확인하기 어려울 것이다.

피드백 이후 각 상수를 표현하기에 적절한 객체는 없는지 고민했고, 상수를 관련 클래스에 포함시켰다. (Lotto.SIZE 등)

 

 

모델 클래스와 테스트 클래스를 1:1 구조로 만들기

model 클래스가 많지 않고 테스트 개수도 많지 않아서, ModelTest라는 클래스에서 모든 모델을 테스트했다.

그런데 model 클래스의 개수가 많아진다면, ModelTest는 수만 라인이 될 수 있다.

즉, model 클래스와 그 model에 대한 테스트 클래스를 1:1 대응되도록 해야 한다.

(추가적으로, 테스트 코드는 구현 코드와 패키지를 동일하게 위치시킨다.)

 

 

일급 컬렉션 내부의 프로퍼티를 노출시킬지

List<LottoNumber>를 감싸는 Lotto를 구현했다.

Lotto안에 number라는 List<LottoNumber> 타입의 프로퍼티가 있었다.

lotto.numbers.contains(LottoNumber(2))
lotto.contains(LottoNumber(2))

외부에서 이 Lotto 객체를 사용하기 위해 위 코드처럼 작성할 수 있다.

그런데 나는 1번 코드보다는 2번 코드를 사용하고 싶었다.

외부에서 Lotto 객체가 numbers 프로퍼티를 알 필요가 없고, 가독성 또한 좋기 때문이다.

하지만 2번 코드를 사용하기 위해서는 contains와 같은 함수를 구현해야 한다.

이에 대해 리뷰어님의 생각이 궁금해서 질문을 드렸다.

 

리뷰어님은 외부로 numbers를 드러내지 않게 설계한 것은 좋지만,

추후 너무 많은 함수가 만들어질 수 있어 유지보수성이 떨어질 수 있다고 하셨다.

이는 trade-off가 있기 때문에 큰 규모의 프로젝트에서 경험해보면 된다고 말씀해주셨다.

(by 를 통해 위임 패턴을 구현할 수 있지만, 이것 또한 trade-off가 존재..)

 

또한 numbers 프로퍼티를 private으로 유지하기 위해 이 방법을 선택한 것도 있는데,

다른 코멘트에서 일급 컬렉션의 프로퍼티는 무조건 private일 필요는 없다고 하셨다.

불변 객체로 유지가 된다면 public으로 직접 노출되어도 괜찮다.

 

 

적절한 객체에게 책임을 부여하기

가장 중요하지만 가장 어려운 사항이라고 생각한다.

로또 미션을 진행하며 해당 피드백을 정말 많이 받았고, "객체의 역할"에 대해 많이 고민했다.

나는 그동안 "내가 구현하기 편한 설계"를 해왔던 것 같다.

큰 고민없이 내가 편한대로 설계를 했다.

앞으로는 어떤 객체가 어떤 일을 해야 적절할지에 대해 고민하는 습관을 들여야겠다.

 

 

로또 금액을 저장하는 enum class는 상수가 아니다

 

로또 금액을 저장하는 LottoPrize라는 enum class를 생성했다.

이 클래스는 몇등이면 번호 몇개가 맞아야 하고, 얼마의 금액을 받을 수 있는지를 저장한다.

나는 특정한 로직이 없고 값만을 저장하기 때문에 상수라고 생각했다.

그런데 해당 LottoPrize는 도메인이고, 클래스를 표현하기 위한 방법으로 enum이 사용된 것 뿐이라고 하셨다.

즉, 언제든지 요구사항이 변경되면 해당 클래스도 변경될 수 있다는 의미다.

 

 

주생성자에는 의도를 가장 잘 드러나는 프로퍼티를 받는다.

Lotto는 List<LottoNumber>를 감싸는 일급 컬렉션이다.

class Lotto(number: List<Int>)보다는, class Lotto(private val number: List<LottoNumber>)가 적절하다.

만약 List<Int>를 통해 인스턴스를 생성하고 싶다면 부생성자 또는 팩토리 메서드를 활용한다.

 

 

테스트 함수를 구체화하자

as-is : 발행한 로또의 개수를 확인한다()

to-be : 로또 한장의 가격이 1,000원일 때 5,000원을 투입하면 총 5개의 로또가 발행된다()

 

 

테스트하는 클래스와 관련된 로직만 포함한다

A 클래스를 테스트 할 때는, B 클래스가 어떻게 생성되는지 세부 방법은 중요하지 않다.

테스트 코드에는 관심사 로직만 포함시킨다.

나는 A 클래스 테스트 클래스에, B 클래스의 팩토리 메서드를 호출하면서 인스턴스를 생성하는 코드를 길게 작성했다.

테스트 환경에서만큼은 B 클래스의 팩토리 메서드를 호출하지 않고 직접 생성자를 호출해도 된다.

 

 

확장성을 고려한 설계

Lotto.PRICE라는 const val 상수를 선언했다.

로또 하나의 가격은 1,000원으로 항상 고정되어 있다고 생각했기 때문이다.

그런데 만약 로또의 가격이 여러개로 나뉘어진다면 어떻게 될까?

이 경우를 대비해 로또의 가격을 객체의 생성자로 받도록 수정했다.

확장성은 trade-off의 영역이지만 사고를 유연하게 할 수 있는 길이 된다.

 

 

클라이언트 입장에서 함수명을 짓기

Map<K, V>을 감싸는 일급 컬렉션(M)을 생성했다.

외부에서 M[K]처럼 사용하기 위해 operator fun get()을 오버라이드 했다.

그런데 해당 클래스를 사용하는 클라이언트 입장에서는, 이 클래스가 Map을 감싸는지 모른다.

또한 M[K]이 어떠한 값을 반환하는지 예상할 수 없다.

즉, 어떠한 값을 반환하는지를 구체적으로 드러낼 수 있는 함수명을 지어야 한다.

 

 

팩토리 메서드 패턴

팩토리 메서드 패턴은 생성에 대한 여러 방법 중 하나가 될 수도 있고(부생성자의 일종),

시스템적으로 생성을 제약하는 방법이 될 수도 있다.

후자의 경우, 정해진 함수 외에는 생성을 막기 위한 장치이므로 private 생성자로 선언해야 한다.

 

 

 

리뷰어/페어 피드백

리뷰어 피드백
- 현재 커리큘럼에 맞게 미션을 잘 따라오고 있음
- 다소의 과한 욕심이 있어 보이지만 성장에 대한 욕심으로 느껴짐
- 피드백 하나하나에 대한 답변과 해결 커밋을 같이 남겨주셔서 좋았음

 

페어 피드백

- 질문이 많았는데 항상 친절하고 자세히 답변해줌

- 자바의 관점에서 코드를 바라보고, 알고 있는 지식이 많아서 도움이 되었음

- 자기주장이 강한 크루를 좋아하는데, 앞으로도 많이 설득하고 다니길 바람.

 

 

 

로또 미션을 끝내고

왠지 모르게 심적으로 힘들었던 미션이었다.

정리해보니 힘든 점이 무려 세 가지나 있었다.

 

 

1) 내 방식을 설득하기 vs 여러 방식을 수용하기 

 

대부분이 그렇겠지만, 나는 내가 기존에 구현하던 방식이 가장 익숙하게 느껴진다.

그래서 페어에게 내가 왜 이런 방식을 선택했는지 이유를 말했고, 거의 모든 부분에서 설득에 성공했다.

 

그런데 막상 페어가 내 방식을 모두 수용해주니까 마음이 불편하고 미안해졌다.

페어는 여러 방식을 구현해보면서 장단점을 직접 경험해보고 싶어했고, 그래서 내 방식을 수용한 것이었다.

(물론 납득 가능한 이유였기 때문도 있을 것이다)

 

내 선택의 이유를 정확히 알고 있고 이를 논리적으로 말하는 것도 좋지만,

매번 같은 방식으로만 구현한다면 과연 얻어가는 것이 있을까?

이번 미션에서 처음으로 내 고집이 강한가?라는 생각을 하게 되었다.

 

 

2) 나는 잘 하는 사람이 아니었구나!

 

그동안 나는 "신입 개발자치고는 잘 하는 사람"이라고 생각해왔다.

나름 수석 졸업도 하고 알고리즘도 열심히 했었고, 대기업 인턴도 했었기 때문이다.

 

하지만 우테코 내에서는  잘 하는 편이 아니었다.

이번 미션에서 모든 크루의 PR을 읽어봤다.

설계에서 막히는 부분들이 정말 많았고, 크루들의 코드를 보면서 배우기 위해서다.

PR을 보면서 내가 설계를 못하는 편이라는 걸 계속 느꼈다.

내가 잘 하는 편이 아니라는 것을 몸소 느낄 때마다 굉장한 자괴감에 빠지는 것 같다.

 

이제는 이것을 기회로 삼으려 한다.

부끄럽지만 그동안 나는 수석 졸업, 알고리즘, 인턴 등등.. 을 모두 해낸 나!에 얽매여 있었다.

주변에 나보다 잘하는 사람이 많다는 것은, 그만큼 배울 것이 많다는 것과 같다.

우테코에서는 순위, 점수, 경쟁이 없는 만큼, 남들과 비교하는 안 좋은 습관을 떨쳐내려 한다.

 

 

3) 심신 미약 이슈..

 

이번 미션에서 겪었던 마지막 어려움은.. 리뷰어의 차가움이다.

당연히 리뷰어 마다 말투가 다른데, 이번 리뷰어는 정말 극강의 T이신 것 같다.

위에서 적었듯 내가 못하는 사람이라는 걸 깨달으면서 자신감이 많이 떨어졌었는데,

코드에 대한 칭찬도 거의 받지 못하니 자존감이 바닥을 찍었다.

 

이걸 적고있는 지금은 ㅋㅋㅋ (당연하게도) 리뷰어의 말투 때문이 아닌, 내 심신미약으로 인한 힘듦이었다는 것을 안다.

당시에는 힘들었어도, 막상 돌이켜보면 배워갈 게 정말 많았다.

코멘트가 많이 달리고 고칠 부분이 많다는 것은

내가 못한다고 주눅들 것이 아니라, 배워갈 점이 많다는 것이다. 

 

step 2 merge 됐을 때 남겨주신 코멘트로 마무리 해야겠다..

리뷰어 입장에서 리뷰하는 즐거움이 있었다니!

그동안의 힘듦은 모두 사라지고 정말 기쁜 순간이었다.

 

앞으로 리뷰어와 코멘트를 주고받을 일이 많을텐데, 이 사실을 꼭 상기시켜야겠다.

지금은 엄청난 성장의 기회라는 것!!!!!

 

728x90
250x250
공지사항
최근에 올라온 글
최근에 달린 댓글
Total
Today
Yesterday
링크
TAG
more
«   2024/10   »
1 2 3 4 5
6 7 8 9 10 11 12
13 14 15 16 17 18 19
20 21 22 23 24 25 26
27 28 29 30 31
글 보관함