[TDD, 클린 코드 with Java] 2.로또 - TDD

* 개요

2단계 로또 TDD 과제를 하면서 피드백 받은 부분을 정리합니다

 

1단계 - 문자열 계산기(로또 - TDD)

메서드명의 의도를 분명하게 드러내라

 

 메서드명을 단순하게 main으로 하지 않고 분명하게 의도가 드러나야 합니다.

 

 

View에서 유효성 검증을 하지 마라

 

 View는 말 그대로 데이터의 입출력만 관리합니다. 유효성 검사는 Controller나 도메인에서 합니다.

 

 

Map은 O(n) 보다는 O(1) 을 고려하라

 

 지금은 Map 원소의 갯수가 많이 없지만, 갯수가 많아지면 O(n)의 성능이 문제가 될 수 있습니다. 따라서 미리 선언하여 O(1)로 쉽게 검색이 가능하도록 합니다.

 

 

@DisplayName으로 테스트 설명을 추가하라

 

 테스트명으로 충분한 의도를 나타낼 수 있지만, @DisplayName을 사용해 좀 더 명확하게 의도를 드러내도록 합니다.

 

 

객체의 필드는 언제 사용해야 하는지 고민

 

 객체 필드는 상태가 있거나, 의미있게 쓰이는 경우 사용합니다. 메서드가 1가지 일만 하기 위해 결과를 조회하기 위한 result()와 계산하는 calculate()를 분리하고 싶었습니다. 그렇다면 값을 저장 할 변수가 필요하고 내부적으로 result 변수를 가지도록 했습니다.

 

 리뷰어는 result가 단순히 결과를 저장하는 용도로만 사용되고 의미있게 사용되지 않는다고 봅니다. 또한 단순히 계산으로 끝나지 않고 calculate() -> result() 불필요한 순서가 필요하다고 봅니다.

 

 이번 과제가 단순하게 기능 구현과 테스트라면 충분히 좋은 리뷰라고 생각합니다. 하지만 객체지향 관점에서 연습하다고 한다면, 각 객체는 자신의 역할이 있고 1가지 메서드는 1가지 역할을 명확하게 드러내며, 메서드 동작은 필드 값을 변화시키는 동사의 개념과, 원하는 필드나 결과 값을 조회하는 명사의 개념을 고려해서 연습해도 좋다고 생각합니다.

 

2단계 - 로또(자동)(로또 - TDD)

 

input의 역할을 축소하라

 

 input에서 입력 및 도메인 생성까지 하는 부분은 너무 많은 역할을 가지고 있습니다. 차라리 input 받은 문자열이나 데이터를 매개변수로 넘겨 도메인을 생성합니다.

 

 

로또 번호 스스로 책임을 관리하는 객체로 분리하라

 

 일반적으로 Integer를 사용해 숫자값을 표현하지만, 로또는 1~45 숫자만 가능한 경우들이므로 별도의 책임을 가지는 객체로 분리합니다. 스스로 유효성 검사를 할 수 있어 객체로 구분될 수 있으며 유연한 코드 작성이 가능합니다.

 

 

무작위 로또 책임은 Lotto 밖으로 분리하라

 

 로또 번호 생성은 별도의 알고리즘 작업을 수행합니다. 따라서 순서에 따라 여러 개의 작은 메서드로 쪼갰습니다. 그런데 Lotto()로 번호를 생성한다면 제대로 로또 번호가 잘 만들어졌는지 알 수 없습니다. 따라서 꼭 테스트를 작성하는 것이 좋습니다. 숫자가 6개인지, 1~45 범위는 유효한지 검사를 해야 안전하게 생성 할 수 있습니다.

 더 명확하게 구분하고 싶다면, 로또 번호 생성 작업을 아예 별도 클래스를 분리 할 수 있습니다. 다양하게 구현될 수 있으므로 수정도 편하고 훨씬 쉽게 테스트 할 수 있으며 의도를 명확하게 드러낼 수 있습니다. 정책이 바뀌어도 도메인은 영향을 덜 받습니다.

 

 

메서드명의 의도를 분명하게 드러내라

 

 나는 맞다고 생각했지만, 다른 사람 입장에서 메서드명을 보고 기능을 혼동할 수 있습니다. 메서드명과 반환형을 같이 읽었다면 좀 더 이해가 쉬웠을 것입니다. 하지만 누군가는 메서드명 기준으로 파악하기 때문에 메서드명 짓기부터 잘 되어야 합니다. 객관적으로 다시 한번 생각해 볼 수 있도록 점검해봅니다.

 

 

컬렉션을 매개변수로 주고받지 마라

 

 merge를 통해 key에 해당하면 value를 1증가시키도록 했습니다. 그렇다면 key에 따라 value가 몇개인지 편하게 계산 할 수 있습니다. 하지만 직접적으로 Map 자체를 put 하는 작업은 복잡성을 증가시킬 가능성이 큽니다. 중간중간 변경되는 경우에도 어디에서 변경되었는지 일일이 파악하는 것이 쉽지 않습니다. 따라서 Map을 꼭 사용하고 싶다면 별도의 클래스로 감싸서 추가시키는 편이 낫습니다.

 

 

상수 네이밍 컨벤션을 적용하라

 

  상수는 Map이든 어떠한 자료형이든 대문자와 밑줄만 사용합니다.

 

 

 예외는 신중하게 사용하라

 

 예외는 비싼 연산입니다. 정확한 갯수 매칭이 올바른 시스템이라고 생각했지만 꼭 예외를 던지지 않아도 비지니스 로직을 이어갈 수 있다면 미당첨 등의 방법으로 이어가는 것도 효율적입니다. Optional의 경우 ofNullable()은 안티패턴으로 차라리 빈 컬렉션을 반환하거나 값 비교는 null 사용이 낫습니다.

 

 

 

클래스 내에 필드의 필요성을 고려하라

 

 결과 값을 필드로 가져 조회 할 수 있게 할 것인가, 메서드가 리턴 값을 가져서 바로 반환 할 것인가 고민해 보았습니다. 혹시 해당 결과를 여러 번 반환해야 한다면 캐싱 개념으로 한번만 작업하고 게속 reateOfReturn을 조회하면 좋겠다고 생각했습니다.

 

하지만, 만약에 결과 값 계산 없이 바로 rateOfReturn을 조회하면 0 값을 얻습니다. 이는 오류로 이어질 수 있습니다. 따라서 바로 결과를 리턴할 수 있도록 변수를 제거합니다. 내부적으로 CPU연산이 있지만 큰 작업이 아니면 여러 번 호출되어도 상관 없습니다.

 

자료형 포장을 고려하라

 

 단순하게 결과를 int 값으로 받으면 저장과 사용에 한계가 있습니다. 명확하게 결과 값인지 구분 하기도 힘듭니다. 따라서 별도로 결과를 저장하는 LottoMatch를 통해 의도를 명확히 드러내고 유효성도 검사 할 수 있습니다.

 

 

3단계 - 로또(2등)(로또 - TDD)

한 메서드 안에 라인수를 15줄 내로 유지하라

 

 아직 리팩토링 전으로 각종 input과 결과 계산, output이 모여 있어서 15줄을 아슬아슬하게 지키고 있습니다. 이는 적절하게 Controller의 생성으로 개선합니다.

 

 

버그 발생 최소화로 안전한 코딩을 작성하라

 

 matchCount가 2인 경우에만 보너스에 영향을 줍니다. 매개변수로 넘어오는 isBonus에도 철저하게 로직 검사를 했기 때문에 버그를 유발 할 가능성이 적다고 생각했습니다. 하지만, 만에하나 잘못 수정이 된다면 오류가 발생할 가능성은 얼마든지 있습니다. 따라서 2등인 경우만 별도로 검사하고 이후에는 isBonus를 사용하지 않도록 수정합니다. 

 

 

 

 

필요하지 않은 변수는 제거하라

 

로또 당첨은 3개부터 의미가 있기 때문에 1개와 2개는 별로 중요하지 않습니다. 하지만, 추후에 1개와 2개인 경우에도 유효 할 경우를 대비해 미리 작성을 하는 것도 딱히 문제가 있지는 않다고 봅니다.

 

 

 

4단계 - 로또(수동)

매직 넘버를 피하라

 

 

 100, 1000이 직관적이라고해도 어떤 의미가 있는지 상수로 선언해 매직넘버를 피하도록 합니다.

 

 

long은 3자리마라 밑줄을 활용하라

 

 long은 3자리마다 밑줄을 그어 활용할 수 있으므로 가독성을 위해 사용합니다.