Step1 - 문자열 계산기 #3897
Step1 - 문자열 계산기 #3897
Conversation
- step1.md 문서 테스트 목록 수정
- 절차적 코드를 클래스 분리하고 각 객체의 역할에 맞게 테스트코드 분리
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class Calculator { |
There was a problem hiding this comment.
공백으로 나눈 토큰 값을 가지는 객체와 그 토큰 리스트를 가진 일급컬렉션에게 메세지를 보내는 방식으로 구현하다보니, 뭔가 Calculator와 Tokens의 사이가 다소 어색해진 것 같기도 합니다.
제 생각엔 이름때문에 그러신 것 같아요 😃
일급 컬렉션을 활용하기 위해 접근하신 방법은 좋다고 생각해요 👍
Calculator의 내부에는 도메인 객체를 생성하고 위임하는 역할을 하고 있는데요
클래스의 이름에 Controller, Application 같은 이름이 있었다면 덜 어색했을 것 같아요.
이름이 계산기이니 직접 계산을 해야할 것 같은 느낌을 지울 수가 없는 것 같아요.
There was a problem hiding this comment.
아, 그렇네요. 이름을 한 번 짓고 나면, 자꾸 코드와 이름이 서로 안맞을 때, 이름을 고친다는 생각을 하기 보다는 코드를 이름에 맞게 다시 수정해야될 것 같은 충동을 느끼는 것 같습니다 😃
적당한 명명일지는 모르겠지만, CalculateController로 변경했습니다.
| @@ -0,0 +1,5 @@ | |||
| package calculator; | |||
|
|
|||
| public class Operator { | |||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class Token { |
There was a problem hiding this comment.
TDD 사이클로 코드를 구현하려고 노력했고 기능을 구현하긴 했으나, 매우 절차 지향적인 코드가 되어서, 한 번에 대규모의 리팩토링을 거쳤습니다.
리팩토링 과정에서 Token 개념이 추가되었네요 👍
커밋 메시지나 readme에 Token이 도출된 이유나 설명을 추가해 주셨으면 더 좋았을 것 같아요.
There was a problem hiding this comment.
커밋 메세지에 나타내는 게 좋겠네요! 도출 이유는 readme에 추가해두었습니다.
| public static final String OPERAND_PATTERN = "[0-9]+"; | ||
| public static final String OPERATOR_PATTERN = "[+\\-*/]"; |
There was a problem hiding this comment.
두 상수만으로 책임이 2개라고 볼 수 있는데요.
숫자와 연산자를 Token으로 추상화할 수 있지만 그 장점이 드러나지 않은 것 같아요.
책임을 분리하면 어떨까요?
|
|
||
| public static final String OPERAND_PATTERN = "[0-9]+"; | ||
| public static final String OPERATOR_PATTERN = "[+\\-*/]"; | ||
| private static final Map<String, Operation> OPERATIONS_MAP = new HashMap<>(); |
There was a problem hiding this comment.
변수명에 자료구조명은 없어도 될 것 같아요
| private static final Map<String, Operation> OPERATIONS_MAP = new HashMap<>(); | |
| private static final Map<String, Operation> OPERATIONS = new HashMap<>(); |
|
|
||
| @Override | ||
| public int perform(int operand1, int operand2) { | ||
| return operand1 + operand2; |
| public interface Operation { | ||
|
|
||
| int perform(int operand1, int operand2); |
There was a problem hiding this comment.
변수명에 넘버링보다 의미 있는 이름을 부여하면 어떨까요?
자바에서 제공하는 IntBinaryOperator 라는 인터페이스가 있으니 활용해 보셔도 좋을 것 같아요
| @DisplayName("두 숫자의 덧셈 연산이 잘 수행된다.") | ||
| @Test | ||
| void addCorrectly() { | ||
| int result = Calculator.calculate("2 + 3"); |
There was a problem hiding this comment.
실제로 덧셈을 수행하는 객체는 Addition 입니다.
AdditionTest를 추가하면 어떨까요?
| @DisplayName("여러 숫자의 복합적 사칙연산이 잘 수행된다.") | ||
| @Test | ||
| void complexCalculateCorrectly() { | ||
| int result = Calculator.calculate("10 + 5 - 5 / 2"); | ||
| assertThat(result).isEqualTo(5); | ||
| } |
There was a problem hiding this comment.
Tokens에서 실제 연산을 수행하고 있어서, CalculatorTest의 내용들을 Tokens로 옮겨야될지 고민이 됩니다.
위와 같은 이유로 TokensTest에 작성할 테스트코드가 마땅치 않고, 그래서 현재는 작성하지 못한 상태입니다
결국 Calculator, Tokens의 역할을 명확하게 구분짓지 못하셔서 고민하시는 것 같아요.
단위 테스트는 실제 로직이 있는 메서드를 대상으로 작성해 주시면 될 것 같아요
| ## 테스트 설계 | ||
| - 입력받은 문자열을 검증한다. | ||
| - [x] 입력받은 문자열이 null 이면 IllegalArgumentException을 반환한다. | ||
| - [x] 입력받은 문자열이 빈 문자열 이면 IllegalArgumentException을 반환한다. | ||
| - 공백을 기준으로 입력받은 문자열을 분리한다. | ||
| - [x] 두 숫자의 덧셈 연산이 잘 수행된다. | ||
| - [x] 두 숫자의 뺄셈 연산이 잘 수행된다. | ||
| - [x] 두 숫자의 나눗셈 연산이 잘 수행된다. | ||
| - [x] 두 숫자의 곱셉 연산이 잘 수행된다. | ||
| - [x] 여러 숫자의 여러 사칙연산이 잘 수행된다. | ||
| - [x] +,-,*,/ 이외의 연산자일 경우 IllegalArgumentException을 반환한다. | ||
| - [x] 피연산자가 숫자가 아닐 경우 IllegalArgumentException을 반환한다. |
There was a problem hiding this comment.
기능 목록을 잘 정리해 주셨네요 👍
각 항목별로 클래스를 도출하셨다면 클래스 단위로 다시 정리해 보셔도 좋을 것 같아요.
TDD 이전에, 객체지향적으로 설계하고 구현하고, 기능 요구사항을 분리하는 것부터 서툰 감이 있고, 많이 부족하다고 느끼고 있습니다.
지금처럼 우선 구현하고 이리저리 리팩토링 하시는 방법 매우 좋습니다 👍
리팩토링할 때에는 커밋 메시지에 '왜 했는지'를 꼭 넣어주시면 더 좋을 것 같아요
참고하실 부분
위와 같은 이유로 TokensTest에 작성할 테스트코드가 마땅치 않고, 그래서 현재는 작성하지 못한 상태입니다.
TDD 이전에, 객체지향적으로 설계하고 구현하고, 기능 요구사항을 분리하는 것부터 서툰 감이 있고, 많이 부족하다고 느끼고 있습니다.
사소한 부분까지도 편히 리뷰해주시면 감사하겠습니다!
잘 부탁드립니다! 😄