문자열 계산기 미션 🧮 #1
문자열 계산기 미션 🧮 #1
Conversation
- 명시된 구분자를 사용하였는지 판별한다.
- 명시된 구분자를 사용하였는지 판별합니다.
- 구분자를 추출하여 사용할 수 있는 구분자인지 검증합니다.
- 추출된 구분자가 사용할 수 있는 구분자인지 확인하는 기능을 추가하였습니다. - 구분자가 사용할 수 없는 구분자일 경우 예외를 발생시키도록 수정하였습니다.
- 문자열에서 변환할 때 음수일 경우 예외가 발생한다.
- 전달받은 구분자를 통해 입력받은 문자열을 구분하는 기능입니다.
- 사용할 수 없는 커스텀 구분자를 검증하는 테스트를 추가하였습니다.
- 커스텀 구분자로 사용할 수 없는 문자열을 명시하여 검증합니다.
- 커스텀 구분자를 정상적으로 추출합니다.
- 하나의 문자열이 아닌 여러 개의 문자열을 파싱할 수 있도록 수정하였습니다.
- model 패키지를 추가하여 관련된 클래스들을 이동하였습니다.
- 기존에는 main에서 실행과 흐름 제어를 담당하고 있었는데, controller가 흐름 제어를 담당하도록 변경하였습니다.
- 기본 구분자를 사용할 때도 추출을 하였으나, 기본 구분자일 경우에는 추출하지 않는 로직으로 변경하였습니다. - 구분자를 위한 상수들을 분리하였습니다.
- 커스텀 구분자 형식 (`//{delimiter}\n`)을 검증하는 테스트 케이스를 추가하였습니다.
- 커스텀 구분자를 추출하기 전 형식이 올바른지 검증하는 로직을 구현하였습니다.
YehyeokBang
left a comment
YehyeokBang
left a comment
There was a problem hiding this comment.
문자열 계산기도 쉽지 않은데, 고생하셨어요! 😄
테스트 커버리지도 잘 맞춰주셨네요. 👍 👍
지윤님의 생각을 물어보는 리뷰가 꽤 있어서 이야기를 더 해보면 좋을 것 같아요.
PR 코멘트
TDD를 사용하다가도 어느 순간 코드부터 수정하고 있어서 당황했습니다 😂
그래도 의식적으로 사용하려고 하니 조금씩 익숙해져서 TDD의 장점을 느낄 수 있었습니다. 문자열 계산기 뿐만 아니라 프로젝트를 진행할 때도 사용해보면서 또 다른 장점이 있는지 궁금해졌습니다.
구체적으로 어떤 장점을 느끼셨나요? 😄
각 클래스 별로 정상적인 상황과 예외의 상황을 단위 테스트를 진행하다보면 통합 테스트에서도 동일한 예외 상황을 테스트하게 되어 중복된 테스트 코드가 생겨납니다.
중복된 테스트 코드는 무엇이 문제인가요? 중복 그 자체가 문제라면, 한번 테스트된 부분은 테스트 하지 않아야 할까요?
| # java-calculator-precourse | ||
|
|
There was a problem hiding this comment.
커밋 메시지에 관한 지윤님만의 기준(단위 및 시제, 어조 포함)이 있으면 좋을 것 같아요. 😄
커밋은 작성 당시의 변경 사항을 설명하는 게 아니라 왜와 무엇이 바뀌었는지 기록하는 것이다. 라는 기준이 있다고 가정하면,
단순한 서술보다 의도를 명확히 하는 데 중점을 둘 수 있을 것 같아요. (중요한건 일관성!)
커밋 메시지 본문에 판별한다., 판별합니다., 추가하였습니다.
시제와 어조가 혼재되어 있어 통일하면 더 좋을 것 같아 리뷰 남깁니다! 👍
There was a problem hiding this comment.
- 사용 안내가 더 위에 출력되면 좋을 것 같아요. 덧셈할 문자열을 아래로!
- 커스텀 구분자를 어떻게 사용할 수 있을까요?
{}는 무엇을 의미하나요?- 문서를 읽지 않고 바로 실행하더라도 사용하기 쉬우면 더 좋을 것 같아요.
There was a problem hiding this comment.
커밋 메시지에 관한 지윤님만의 기준(단위 및 시제, 어조 포함)이 있으면 좋을 것 같아요. 😄
커밋은 작성 당시의 변경 사항을 설명하는 게 아니라 왜와 무엇이 바뀌었는지 기록하는 것이다. 라는 기준이 있다고 가정하면,
단순한 서술보다 의도를 명확히 하는 데 중점을 둘 수 있을 것 같아요. (중요한건 일관성!)
커밋 메시지 본문에 판별한다., 판별합니다., 추가하였습니다.
시제와 어조가 혼재되어 있어 통일하면 더 좋을 것 같아 리뷰 남깁니다! 👍
커밋 메세지를 일관적이게 작성했다고 생각했는데 놓친 부분이 있었군요 😅 조금 더 신경써서 작성하겠습니다. 감사합니다 :)
사용 안내가 더 위에 출력되면 좋을 것 같아요. 덧셈할 문자열을 아래로!
커스텀 구분자를 어떻게 사용할 수 있을까요? {}는 무엇을 의미하나요?
문서를 읽지 않고 바로 실행하더라도 사용하기 쉬우면 더 좋을 것 같아요.
헉 저 {구분자}는 {}를 제외한 구분자만 입력하는 걸 표현한 것이었는데 혼동이 있을 수 있겠네요! 수정하겠습니다.
어떤 차이가 있을까요?
기본 구분자 사이의 공백을 제대로 처리하지 못하고 있는 것 같아요 🥺 수정하겠습니다!
|
|
||
| - 기본 구분자인 쉼표`,`나 콜론`:`을 구분자로 가지는 문자열을 받으면 구분자를 기준으로 숫자를 분리한 후 숫자를 더한 값을 반환한다. | ||
| - 기본 구분자 외에 커스텀 구분자를 `//`와 `\n`에 정의하여 사용할 수 있다. | ||
| - 잘못된 값을 입력할 경우 `IlLegaLArgumentException`을 발생시킨 후 종료된다. |
There was a problem hiding this comment.
| - 잘못된 값을 입력할 경우 `IlLegaLArgumentException`을 발생시킨 후 종료된다. | |
| - 잘못된 값을 입력할 경우 `IllegalArgumentException`을 발생시킨 후 종료된다. |
| ## 예외 예제 | ||
|
|
||
| | 입력 | 예외 (`IllegalArgumentException`) | | ||
| |---------------|---------------------------------| | ||
| | `1,2,-3` | `양수를 입력해주세요` | | ||
| | `1,2;3` | `기본 구분자(,나 :)를 사용해주세요.` | | ||
| | `1,a,3` | `숫자만 입력해주세요.` | | ||
| | `//;1;2;3` | `커스텀 구분자 형식에 맞게 입력해주세요.` | | ||
| | `//\n123` | `커스텀 구분자를 입력해주세요.` | | ||
| | `//1\n112131` | `사용 가능한 커스텀 구분자를 입력해주세요.` | |
There was a problem hiding this comment.
지윤님은 어떤 장점을 위해 예외 상황을 문서로 관리하는 것을 선택하셨나요?
예외를 문서로 작성하고 관리하게 되면 어떤 장점과 단점이 있을까요?
There was a problem hiding this comment.
README를 보고 제가 생각한 예외 상황에 대해서 명시하기 위해서 작성했습니다!
하지만 작성한 예외 상황 외에도 더 많은 상황들이 발견될 때마다 README에 업데이트 해줘야 하는 단점이 있을 것 같아요,,
그렇다면 예혁님은 테스트 코드로만 예외 상황을 표현하시나요?
README에 '이건 제외하는 게 좋다'하는 기준이 있으시면 알려주실 수 있을까요?
There was a problem hiding this comment.
README를 보고 제가 생각한 예외 상황에 대해서 명시하기 위해서 작성했습니다!
요구사항을 보고 생각했던 예외를 말씀 하신 걸까요? 그리고 어떤 장점이 있었기에 명시하려고 하셨나요?
그렇다면 예혁님은 테스트 코드로만 예외 상황을 표현하시나요?
README에 '이건 제외하는 게 좋다'하는 기준이 있으시면 알려주실 수 있을까요?
README는 결국 필요한 내용을 담는 것이 가장 큰 기준이 되지 않을까 싶어요.
예를 들어, 애플리케이션을 처음 실행하는 사람을 위해 환경 설정이나 실행 방법을 작성할 수도 있고,
처음 보는 사람도 어떤 프로그램인지 빠르게 이해할 수 있도록 실행 화면을 캡처해 첨부할 수도 있겠죠.
저는 이런 건 넣고, 저런 건 빼야 한다는 고정된 기준보다는
README를 읽는 사람의 입장에서 어떤 것을 도울 수 있을까?를 생각해보면서
목적를 중심으로 구성하면 더 좋다고 생각해요! 😄
| public class DelimiterConstants { | ||
| public static final Pattern DEFAULT_DELIMITER_REGEX = Pattern.compile("^[0-9,:]+$"); | ||
| public static final String CUSTOM_DELIMITER_PREFIX = "//"; | ||
| public static final String CUSTOM_DELIMITER_SUFFIX = "\\n"; | ||
| } |
There was a problem hiding this comment.
// 1. 이벤트용으로 사용할 상수를 만들 때 상속
class EventDelimiterConstants extends DelimiterConstants {
public static final String EVENT_DELIMITER = "event";
}
// 2. 인스턴스화
DelimiterConstants ds = new DelimiterConstants; // 가능DelimiterConstants를 상속하여 새로운 상수를 추가(EventDelimiterConstants)하거나
상수 정의를 확장할 수 있어요. 이것은 의도된 구조인가요?
DelimiterConstants는 기본 생성자를 통해 인스턴스화가 가능하네요.(new DelimiterConstants()).
의도가 상수 저장용 유틸리티 클래스라면, 인스턴스화를 방지하는 것이 좋을 것 같은데 어떻게 생각하시나요?
There was a problem hiding this comment.
DelimiterConstants를 상속하여 새로운 상수를 추가(EventDelimiterConstants)하거나
상수 정의를 확장할 수 있어요. 이것은 의도된 구조인가요?
의도하지 않았습니다,,😢 예혁님 말대로 상수 저장을 위한 클래스이기 때문에 인스턴스화 방지를 하는게 맞다고 생각합니다!
| public List<String> extract(final String input) { | ||
| if (isCheckCustomDelimiterFormat(input)) { | ||
| validateCustomDelimiterFormat(input); | ||
| return extractCustomDelimiters(input); | ||
| } | ||
|
|
||
| validateDefaultDelimiter(input); | ||
| return Delimiter.getDefaultDelimiter(); | ||
| } |
There was a problem hiding this comment.
메서드 내에서 줄바꿈을 사용하는 지윤님의 기준을 설명해주실 수 있나요?
- 이 메서드는 if 내의 return 문 위에는 줄바꿈이 없어요.
- if 아래에 줄바꿈이 있어요. (
DelimiterExtractor#validateCustomDelimiterFormat메서드에는 if 아래에 줄바꿈이 없어요.)
- if 아래에 줄바꿈이 있어요. (
- 이 메서드 끝 return 문 위에 코드 한줄이 있어요.
DelimiterExtractor#extractCustomDelimiters메서드에서는 끝나는 return문 위에 바로 줄바꿈이 있어요.
There was a problem hiding this comment.
문맥상 연관이 있다고 생각한 부분을 줄바꿈 없이 작성한 것 같아요.
근데 다른 코드에서는 return 문 위에 줄바꿈을 한 것이 있고 안한 것이 있네요 !
컨벤션을 의식적으로 신경쓰면서 일관적이게 코드 수정해보겠습니다. 감사합니다
| public int add(List<Integer> values) { | ||
| int result = 0; | ||
| for (int value : values) { | ||
| result += value; | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
현재 코드에서 이 메서드의 역할은 리스트의 값을 모두 더해 최종 합계를 반환하는 것 같아요! 😄
add는 일반적으로 값을 하나씩 추가하거나 덧셈의 동작을 명확히 나타내는 데 사용되는 것으로 알고 있어요.
예를 들어, 리스트에 값을 추가한다 (list.add(5))거나 단일 덧셈 작업 등이 있어요.
반면, sum은 현재 메서드처럼 컬렉션의 모든 값을 합산하여
최종 결과를 계산하는 작업을 표현할 때 주로 사용돼요.
예시: int total = numbers.stream().mapToInt(Integer::intValue).sum();
이 메서드는 결국 최종 합계의 개념을 전달하므로,
결과 값 중심의 맥락을 유지하기 위해 sum이라는 이름이 더 자연스러울 것 같은데 어떻게 생각하시나요? 😄
// total을 만드는 과정이 sum
for (int value : values) {
total += value; // add
}There was a problem hiding this comment.
덧셈이니 add라고 생각했는데 예혁님이 말씀해주신 대로 sum이 더 나을 것 같아요! 코드로 설명해주시니 더 이해가 쉽게 되네요 👍🏻 감사합니다.
| private boolean isCheckedEmptyOrNull(final String input) { | ||
| if (input == null || input.isBlank()) { | ||
| OutputView.printResult(0); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
입력값이 빈문자열인 경우 아무 연산을 수행하지 않고, 바로 0을 반환하도록 구성한 것은 효율적인 방법일 수 있어요. 👍 👍
그러나 유지보수하기 어려울 수도 있겠다는 생각이 들었어요.
예를 들어, 숫자를 1만 입력하면 그대로 1을 출력하도록 하는 것도 Controller에서 처리해준다면 여러 계산이나 구분자 처리 없이 가능하지 않을까요? 0으로 반환 또는 특수한 입력을 처리하는 로직이 늘어난다면 어떻게 될까요?
저는 오히려 예상하지 못한 곳에 코드가 존재하게될 확률이 높아질 것 같아요.
아무것도 입력하지 않으면 어떻게 처리되지? 계산기 코드를 봐야겠다 -> 찾을 수 없음
Controller가 아닌 모델이 책임질 수 있도록 설계하면 어떨까요?
There was a problem hiding this comment.
저도 이 의견에 동의하는 것 같아요. 아무것도 입력하지 않았을 때 최대한 적은 접근으로 반환하려 하다보니 Controller에 두게 됐네요!
예혁님이 말씀해주신 부분 참고해서 수정해보겠습니다. 감사합니다 🙇🏻♀️
| public class CalculatorController { | ||
|
|
||
| private final DelimiterExtractor delimiterExtractor; | ||
| private final StringSplitter stringSplitter; | ||
| private final Calculator calculator; | ||
|
|
||
| public CalculatorController(final DelimiterExtractor delimiterExtractor, | ||
| final StringSplitter stringSplitter, | ||
| final Calculator calculator) { | ||
| this.delimiterExtractor = delimiterExtractor; | ||
| this.stringSplitter = stringSplitter; | ||
| this.calculator = calculator; | ||
| } |
There was a problem hiding this comment.
해당 객체들을 Controller의 상태(필드)로 가져야 하는 이유가 있나요?
There was a problem hiding this comment.
흐름을 제어하기 위해 필요하다고 판단해서 객체를 주입하는 방식으로 선택했습니다.
만약 상태를 가지게 하지 않기 위해서는 저 객체들이 상호작용할 수 있도록 Service 클래스가 생길 수 있을 것 같은데 그 부분을 고려해서 수정 하는게 더 나은 설계인가요?
There was a problem hiding this comment.
흐름을 제어하기 위해 필요하다
이 부분이 조금 추상적으로 느껴졌어요. 혹시 어떤 흐름을 말씀하신 건가요?
그 객체(DelimiterExtractor, StringSplitter 등)가 필요했다면,
run() 메서드 안에서 객체를 생성해도 되지 않나요?
꼭 필드로 객체를 들고 있어야 하는가?에 대한 것이 궁금해서 드린 질문이었습니다.
| public enum Delimiter { | ||
| COMMA(",", true), | ||
| COLON(":", true), | ||
| DOUBLE_SLASH("//", false), | ||
| NEW_LINE("\\n", false), | ||
| NUMBER("[0-9]+", false), | ||
| SPACE(" ", false); | ||
|
|
||
|
|
||
| private final String delimiter; | ||
| private final boolean isDefault; |
There was a problem hiding this comment.
COMMA, COLON과 안되는 것을 구분하기 위해, isDefault 필드를 가지게 되었고, 더 복잡해진 것 같아요.
각 요소를 살펴보면 문자열이다. 말고는 공통된 요소가 없는 것 같은데 Enum으로 관리하는 이유가 있나요?
- COMMA, COLON : 기분 구분자 성격
- DOUBLE_SLASH, NEW_LINE : 커스텀 구분자를 자르기 위해 필요한 커맨드
- NUMBER : 안되는 것
- SPACE : 코드에서 사용되지 않음 (필요하지 않다면 삭제해 주세요!)
기본 구분자 열거형, 커스텀 구분자로 사용할 수 없는 구분자 열거형 이런 식으로 각 책임에 맞게 열거형을 선언하는 것은 어떨까요?
There was a problem hiding this comment.
Delimiter를 통합적으로 사용한 것은 커스텀 구분자에 사용할 수 없는 구분자로 정의하기 위함이 우선적이었습니다.
그러다 보니 기본 구분자를 사용한 입력에서 기본 구분자를 판별하는데에서 isDefault로 구분을 해주게 되었습니다.
그래도 구분하는게 더 나을까요?
There was a problem hiding this comment.
생각의 변화 유무의 상관 없이 근거를 가지고 이런 결정을 했다. 라고 제시해주시는 편이 더 좋을 것 같아요. 😄
| public class Calculator { | ||
|
|
||
| public int add(List<Integer> values) { |
There was a problem hiding this comment.
문자열 계산기 프로젝트에서 계산기 객체가 중요할 것 같다고 판단했는데,
숫자를 모두 더하는 기능만 있어서 의외였던 것 같아요.
오히려 Controller가 문자열 계산기 프로젝트 내에서 가장 비즈니스 로직을 많이 수행하는 것 같은데
이에 대해서는 어떻게 생각하시나요? 😄
There was a problem hiding this comment.
저는 각 클래스가 책임에 따라 역할을 명확히 나누는 데 중점을 두었고, Controller는 그 흐름을 제어하는 조립자라고 생각했어요!
하지만, 예혁님 피드백을 보고 Controller에서 핵심 기능인 덧셈이 어떻게 이루어지는 지에 대한 부가적인 기능들을 다 알고 있어도 되는지에 대한 고민이 생겼어요.
그렇다면 Calculator가 비지니스 로직을 수행하는 것이 가장 이상적인 설계일까요?
리뷰를 제가 잘 이해하고 대답한 건지 모르겠네요 .. 😅
There was a problem hiding this comment.
저는 각 클래스가 책임에 따라 역할을 명확히 나누는 데 중점을 두었고, Controller는 그 흐름을 제어하는 조립자라고 생각했어요!
각 클래스가 책임에 따라 역할을 명확히 나누는 데 중점을 두셨고,
Controller를 흐름을 제어하는 조립자로 보셨다고 하셨어요.
지금 구조를 보면 Controller가 여러 객체들을 직접 조립하고 호출하는 방식이에요.
그런데 이 방식이 정말 객체지향적인 협력이라고 볼 수 있을까? 하는 생각이 들었어요.
Controller는 단순히 조립자 이상의 책임을 갖고 있고,
계산의 핵심인 숫자 분리와 덧셈까지 모두 관여하고 있는 구조니까요.
Calculator는 단순히 List를 더하는 역할만 수행하고 있어서,
과연 이게 비즈니스 로직을 담당하는 객체인가?라는 의문도 들었고요.
조금 더 객체들 간에 역할을 분산시키고, 서로 알아서 일할 수 있는 구조로 바꿔보는 것은 어떨까요?
고민을 더 해보시면 좋을 것 같습니다! 👍
테스트 코드를 먼저 작성하면서 스스로 생각한 흐름대로 동작하는 지 확인을 바로 할 수 있었습니다. 그리고 다른 코드를 수정하게 되면서 기존에 작성한 테스트 코드에 영향이 간다면 두 클래스의 결합도에 대해서도 신경쓰면서 고민해볼 수 있었던 것 같아요!
스스로 판단했을 때 이 조건에 맞는 검증이 필요하다면 중복되는 것에 대해서는 문제가 되지 않을 것이라고 생각해요. |
There was a problem hiding this comment.
고생하셨습니다! 😄
어떤 의도를 가지고 작성했는지 이해하기 편했습니다! 👍
답변은 모두 확인했는데, 반영이 안된 것 같아요!
확인 부탁드려요~
답변
테스트 코드를 먼저 작성하면서 스스로 생각한 흐름대로 동작하는 지 확인을 바로 할 수 있었습니다. 그리고 다른 코드를 수정하게 되면서 기존에 작성한 테스트 코드에 영향이 간다면 두 클래스의 결합도에 대해서도 신경쓰면서 고민해볼 수 있었던 것 같아요!
예혁님은 TDD가 정말 좋다고 느낀 순간이 있으신가요? 🤔
저도 리팩토링할 때 이전에 작성했던 테스트 코드의 덕을 많이 봤던 것 같아요.
기존 기능이 깨지지 않았다는 걸 테스트가 보장해주니까, 마음 놓고 구조를 바꿔볼 수 있더라고요.
그리고 가끔은 테스트를 작성하다가 어..? 이렇게 쓰기 불편한데? 하면서
오히려 설계 자체를 더 깔끔하게 바꾸게 되는 경험도 있었어요 😄
TDD가 단순히 테스트 도구가 아니라, 설계 방향을 알려주는 가이드처럼 느껴지는 것 같아요.
물론 장점만 느끼진 않았습니다! 어려운 도메인을 해결하려면 어디부터 테스트를 작성해야 할까? 라는 고민이 크게 드는 것 같아요.
스스로 판단했을 때 이 조건에 맞는 검증이 필요하다면 중복되는 것에 대해서는 문제가 되지 않을 것이라고 생각해요.
스스로 판단했을 때 이 조건에 맞는 검증이 필요하다면 스스로 판단하는 기준이 있나요? 직감인가요?
피드백
- 추상적인 답변을 다듬어 보는 시간을 가지면 좋을 것 같아요.
- 생각의 변화 유무의 상관 없이 근거를 가지고 이런 결정을 했다. 라고 제시해주시는 편이 더 좋을 것 같아요. 😄
| ## 예외 예제 | ||
|
|
||
| | 입력 | 예외 (`IllegalArgumentException`) | | ||
| |---------------|---------------------------------| | ||
| | `1,2,-3` | `양수를 입력해주세요` | | ||
| | `1,2;3` | `기본 구분자(,나 :)를 사용해주세요.` | | ||
| | `1,a,3` | `숫자만 입력해주세요.` | | ||
| | `//;1;2;3` | `커스텀 구분자 형식에 맞게 입력해주세요.` | | ||
| | `//\n123` | `커스텀 구분자를 입력해주세요.` | | ||
| | `//1\n112131` | `사용 가능한 커스텀 구분자를 입력해주세요.` | |
There was a problem hiding this comment.
README를 보고 제가 생각한 예외 상황에 대해서 명시하기 위해서 작성했습니다!
요구사항을 보고 생각했던 예외를 말씀 하신 걸까요? 그리고 어떤 장점이 있었기에 명시하려고 하셨나요?
그렇다면 예혁님은 테스트 코드로만 예외 상황을 표현하시나요?
README에 '이건 제외하는 게 좋다'하는 기준이 있으시면 알려주실 수 있을까요?
README는 결국 필요한 내용을 담는 것이 가장 큰 기준이 되지 않을까 싶어요.
예를 들어, 애플리케이션을 처음 실행하는 사람을 위해 환경 설정이나 실행 방법을 작성할 수도 있고,
처음 보는 사람도 어떤 프로그램인지 빠르게 이해할 수 있도록 실행 화면을 캡처해 첨부할 수도 있겠죠.
저는 이런 건 넣고, 저런 건 빼야 한다는 고정된 기준보다는
README를 읽는 사람의 입장에서 어떤 것을 도울 수 있을까?를 생각해보면서
목적를 중심으로 구성하면 더 좋다고 생각해요! 😄
| public class CalculatorController { | ||
|
|
||
| private final DelimiterExtractor delimiterExtractor; | ||
| private final StringSplitter stringSplitter; | ||
| private final Calculator calculator; | ||
|
|
||
| public CalculatorController(final DelimiterExtractor delimiterExtractor, | ||
| final StringSplitter stringSplitter, | ||
| final Calculator calculator) { | ||
| this.delimiterExtractor = delimiterExtractor; | ||
| this.stringSplitter = stringSplitter; | ||
| this.calculator = calculator; | ||
| } |
There was a problem hiding this comment.
흐름을 제어하기 위해 필요하다
이 부분이 조금 추상적으로 느껴졌어요. 혹시 어떤 흐름을 말씀하신 건가요?
그 객체(DelimiterExtractor, StringSplitter 등)가 필요했다면,
run() 메서드 안에서 객체를 생성해도 되지 않나요?
꼭 필드로 객체를 들고 있어야 하는가?에 대한 것이 궁금해서 드린 질문이었습니다.
| public class Calculator { | ||
|
|
||
| public int add(List<Integer> values) { |
There was a problem hiding this comment.
저는 각 클래스가 책임에 따라 역할을 명확히 나누는 데 중점을 두었고, Controller는 그 흐름을 제어하는 조립자라고 생각했어요!
각 클래스가 책임에 따라 역할을 명확히 나누는 데 중점을 두셨고,
Controller를 흐름을 제어하는 조립자로 보셨다고 하셨어요.
지금 구조를 보면 Controller가 여러 객체들을 직접 조립하고 호출하는 방식이에요.
그런데 이 방식이 정말 객체지향적인 협력이라고 볼 수 있을까? 하는 생각이 들었어요.
Controller는 단순히 조립자 이상의 책임을 갖고 있고,
계산의 핵심인 숫자 분리와 덧셈까지 모두 관여하고 있는 구조니까요.
Calculator는 단순히 List를 더하는 역할만 수행하고 있어서,
과연 이게 비즈니스 로직을 담당하는 객체인가?라는 의문도 들었고요.
조금 더 객체들 간에 역할을 분산시키고, 서로 알아서 일할 수 있는 구조로 바꿔보는 것은 어떨까요?
고민을 더 해보시면 좋을 것 같습니다! 👍
| public enum Delimiter { | ||
| COMMA(",", true), | ||
| COLON(":", true), | ||
| DOUBLE_SLASH("//", false), | ||
| NEW_LINE("\\n", false), | ||
| NUMBER("[0-9]+", false), | ||
| SPACE(" ", false); | ||
|
|
||
|
|
||
| private final String delimiter; | ||
| private final boolean isDefault; |
There was a problem hiding this comment.
생각의 변화 유무의 상관 없이 근거를 가지고 이런 결정을 했다. 라고 제시해주시는 편이 더 좋을 것 같아요. 😄
| private static List<Integer> getNumbers(final List<String> input) { | ||
| return input.stream() | ||
| .mapToInt(Integer::parseInt) | ||
| .boxed() | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
getNumbers에서 문자열을 숫자로 변환하는 책임을 지는 것을 의도했기 때문에
말씀하신 의도가 메서드의 시그니처로 충분히 설명될 것 같나요? 지윤님의 생각은 어떠신가요?
| private String getRegex(final List<String> delimiters) { | ||
| return delimiters.stream() | ||
| .map(Pattern::quote) | ||
| .collect(Collectors.joining("|")); | ||
| } |
There was a problem hiding this comment.
처음보는 개발자가 이해하기 어려울 것 같아요. 상수화를 해보는 것은 어떨까요?
문자열로 쓰이는 |인지,
말씀 하신 정규표현식의 OR인지 이해하기 힘들었습니다!
| public class OutputView { | ||
|
|
||
| private OutputView() { | ||
| } |
There was a problem hiding this comment.
입력과 출력을 나눈다고 하셨는데 지윤님이 생각하시는 입력과 출력의 정의는 무엇인가요?
// InputView 클래스
public static String getInput() {
System.out.println(START_MESSAGE); // 이것도 출력 아닌가요?
System.out.println(USAGE_GUIDE);
return Console.readLine();
}| @ParameterizedTest | ||
| @DisplayName("명시된 구분자와 일치하는 구분자인지 정상적으로 판별한다.") | ||
| @ValueSource(strings = {",", ":"}) | ||
| void shouldReturnTrue_whenValidDelimiter(String delimiter) { | ||
| assertTrue(Delimiter.isValidDelimiter(delimiter)); | ||
| } |
There was a problem hiding this comment.
| @ParameterizedTest | |
| @DisplayName("명시된 구분자와 일치하는 구분자인지 정상적으로 판별한다.") | |
| @ValueSource(strings = {",", ":"}) | |
| void shouldReturnTrue_whenValidDelimiter(String delimiter) { | |
| assertTrue(Delimiter.isValidDelimiter(delimiter)); | |
| } | |
| @ParameterizedTest | |
| @DisplayName("명시된 구분자와 일치하는 구분자인지 정상적으로 판별한다.") | |
| @ValueSource(strings = {",", ":"}) // given | |
| void shouldReturnTrue_whenValidDelimiter(String delimiter) { | |
| // when | |
| var actual = Delimiter.isValidDelimiter(delimiter); | |
| // then | |
| assertTrue(actual); | |
| } |
테스트가 한 줄이라 주석을 생략했다고 하셨는데, 저는 오히려 간단한 테스트일수록
given / when / then을 명확히 나누는 것이 테스트의 의도를 더 잘 드러낼 수 있다고 생각해요.
위처럼 주석을 넣는다면 어떤 입력으로 어떤 동작을 했고, 무엇을 검증하는지를
코드를 처음 보는 사람도 빠르게 이해할 수 있지 않을까요? 😊
특히 한 줄짜리 테스트일수록 흐름이 명확하지 않기 때문에
isValidDelimiter() 호출이 when인지 then인지 애매할 수도 있어요.
이럴 때 주석은 흐름을 분명히 보여주는 좋은 수단이 될 수 있다고 생각해요!
| @ParameterizedTest | ||
| @DisplayName("커스텀 구분자로 사용할 수 있는 구분자일 경우 true를 반환한다.") | ||
| @ValueSource(strings = {";", "#", "@", "a", "*"}) | ||
| void shouldReturnTrue_whenValidCustomDelimiter(String delimiter) { | ||
| assertTrue(Delimiter.isValidCustomDelimiter(delimiter)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @DisplayName("커스텀 구분자로 사용할 수 없는 구분자일 경우 false를 반환한다.") | ||
| @ValueSource(strings = {",", ":", " ", "1", "2"}) | ||
| void shouldReturnFalse_whenInvalidCustomDelimiter(String delimiter) { | ||
| assertFalse(Delimiter.isValidCustomDelimiter(delimiter)); | ||
| } |
There was a problem hiding this comment.
true/false 각각에 대해 테스트하는 건 분기 커버 측면에서도 자연스럽고, 저도 동의하는 것 같아요/
다만 제가 질문을 드린 이유는,
Boolean 메서드가 많아졌을 때 테스트 코드의 양이 기하급수적으로 늘어나는 구조는 유지보수 측면에서 괜찮을까? 하는 고민 때문이었어요.
예를 들어 isXXX() 같은 메서드가 많아지면
정상 값 테스트, 예외 값 테스트 각각의 테스트 메서드가 매번 생기게 되고,
결국 테스트 파일 자체가 너무 세분화되어서 오히려 읽기 어려워질 수도 있겠다는 생각이 들었거든요.
그래서 말씀하신 것처럼 기능별로 테스트를 묶거나,
입력과 기대 결과를 쌍으로 갖는 인라인 테스트를 고려해볼 수도 있지 않을까 싶어요!
@ParameterizedTest
@CsvSource({";,true", "#,true", ",,false", ":,false"})
void 커스텀_구분자_허용_여부를_정상적으로_판별한다(String input, boolean expected) {
assertEquals(expected, Delimiter.isValidCustomDelimiter(input));
}이런 식으로 표현하면 테스트 수는 그대로지만, 구조는 조금 더 압축적일 수 있을 것 같아요 😊
📍 미션 목표
TDD(Test-Driven Development)에 익숙해지는 것을 목표로 하였습니다.🤔 고민했던 부분
기본 구분자와 커스텀 구분자를 어떻게 구분하는 게 좋을지?
생각나는 모든 예제에 대해 방어적인 코드를 작성하는 게 옳은 건지에 대해서 고민했습니다.
명확한 기준을 정하지 않아서 중간 중간 변경되는 코드가 많아졌던 것 같습니다.
통합 테스트와 단위 테스트의 기준을 어떻게 두어야 할 지?
제가 생각하는 차이는 단위 테스트는 클래스의 역할을 적절하게 수행하는지 확인하는 것이고 통합 테스트는 사용할 때의 흐름을 테스트한다고 생각합니다.
하지만, 각 클래스 별로 정상적인 상황과 예외의 상황을 단위 테스트를 진행하다보면 통합 테스트에서도 동일한 예외 상황을 테스트하게 되어 중복된 테스트 코드가 생겨납니다.
어떤 것에 기준을 두고 나누어 테스트를 구성해야할 지 고민입니다.
👀 느낀 점
TDD를 사용하다가도 어느 순간 코드부터 수정하고 있어서 당황했습니다 😂
그래도 의식적으로 사용하려고 하니 조금씩 익숙해져서 TDD의 장점을 느낄 수 있었습니다. 문자열 계산기 뿐만 아니라 프로젝트를 진행할 때도 사용해보면서 또 다른 장점이 있는지 궁금해졌습니다.
항상 '코드에는 정답이 없다'라는 것을 인지하고 선택한 이유를 생각하며 구현하였습니다. 습관적으로 썼던 코드에 이유를 찾으려니 쉽지는 않았지만, 천천히 연습하고 있습니다.