[LBP] 송하은 미션 3,4단계 제출합니다. #72
Conversation
There was a problem hiding this comment.
3,4주차 미션 고생하셨어요 😁😁
미션을 잘 수행하신 것 같아요 고민이 많이하신 것 같은 흔적이 보여서 좋았습니다!
특히 테스트 코드를 잘 작성해두어서 이해하기 쉬웠어요
이번주도 하은이만의 답을 찾아봅시다! 리뷰 확인하고 개선해봅시다!
질문에 대한 답변
현재는, "//;1;2;\n123;" 입력 시에, 123이 커스텀 구분자로 구분되지 않으니 "잘못된 입력 형식입니다." 런타임 예외를 발생하는 코드로 작성하였습니다.
하지만, "123"의 경우에는, 구분자가 없으므로 0으로 반환하는 코드로 작성했습니다.
위 두 가지 경우는 어떻게 처리하는 것이 가장 기능 요구사항과 적합하는지요..!
-
첫번째의 경우 기능 요구 사항에서 정의된 커스텀 구분자 입력 형식에 어긋나보여요
예외를 던지는 것이 요구사항과 적절해보여요 -
두번째의 경우 123 -> 0을 반환하는 것이 맞는가? -> 저는 아니라고 생각해요
123이라는 숫자도 유효한 숫자라고 생각합니다 유효한 숫자이니 합산 대상이 된다고 생각해요
이 부분에 있어서 코멘트에도 작성해두었으니 확인해봐요~
src/main/java/StringCalculator.java
Outdated
| // \n 이후에 최소한 구분자가 한 개 이상은 있어야한다. 구분자가 없다면 배열 길이가 1이된다. | ||
| if(numbers.isEmpty() || numbers.split(separater).length == 1) { | ||
| throw new RuntimeException("잘못된 입력 형식입니다."); | ||
| } |
There was a problem hiding this comment.
해당 조건문에서 "//;\n2"처럼 한 개의 숫자만 입력된 경우 예외가 발생할 가능성이 있어보여요
"2"는 정상적인 입력으로 볼 수 있어야하죠 다시 개선해봅시다!
- "구분자가 하나 이상 포함되어야 한다"는 규칙은 요구사항에 없음
| if (newLineIdx == -1) { | ||
| throw new RuntimeException("잘못된 입력 형식입니다."); | ||
| } | ||
| separater = Pattern.quote(str.substring(2, newLineIdx)); |
src/main/java/StringCalculator.java
Outdated
| int newLineIdx = str.indexOf("\n"); | ||
| // /n 을 찾을 수 없거나, \n 이후 값이 없는 경우 | ||
| if (newLineIdx == -1) { | ||
| throw new RuntimeException("잘못된 입력 형식입니다."); |
src/main/java/StringCalculator.java
Outdated
|
|
||
| public class StringCalculator { | ||
|
|
||
| public int getSum(String str) { |
There was a problem hiding this comment.
하나의 메서드가 너무 많은 책임을 가지고 있어요
코드를 단번에 이해하기에는 어려웠어요 좀 더 분리해볼까요?
There was a problem hiding this comment.
- 입력값 검증
- 구분자 추출
- 문자열을 구분자 기준으로 분리
- 분리된 숫자 문자열 배열을 합산
으로 분리해보았습니다!
src/main/java/StringCalculator.java
Outdated
| public int parseNumber(String number) { | ||
| // 숫자가 아니라면 예외 발생 | ||
| try { | ||
| return Integer.parseInt(number); | ||
| } catch (NumberFormatException e) { | ||
| throw new RuntimeException("숫자가 아닌 값이 포함되어 있습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
외부에서 호출할 필요없는 메서드는 접근제어자를 통해서 캡슐화를 지켜봅시다!
| @DisplayName("문자열 계산기 테스트") | ||
| class StringCalculatorTest { | ||
|
|
||
| StringCalculator stringCalculator = new StringCalculator(); |
There was a problem hiding this comment.
해당 클래스에서만 사용된다면 인스턴스 변수 또한 private로 캡슐화를 지켜봅시다
|
|
||
| StringCalculator stringCalculator = new StringCalculator(); | ||
|
|
||
| @Nested |
| @Test | ||
| @DisplayName("숫자가 아닌 값이 포함되면 예외를 발생해야 한다.") | ||
| void should_throw_exception_when_contains_non_number() { | ||
|
|
||
| // 특수문자가 포함된 경우 | ||
| assertThatThrownBy(() -> stringCalculator.getSum("//;\n1;*;3")) | ||
| .isInstanceOf(RuntimeException.class) | ||
| .hasMessageContaining("숫자가 아닌 값이 포함되어 있습니다."); | ||
|
|
||
| assertThatThrownBy(() -> stringCalculator.getSum("//;\n1456;-;3")) | ||
| .isInstanceOf(RuntimeException.class) | ||
| .hasMessageContaining("숫자가 아닌 값이 포함되어 있습니다."); | ||
|
|
||
| // 글자가 포함된 경우 | ||
| assertThatThrownBy(() -> stringCalculator.getSum("//;\nddd;ad;3")) | ||
| .isInstanceOf(RuntimeException.class) | ||
| .hasMessageContaining("숫자가 아닌 값이 포함되어 있습니다."); | ||
| } |
There was a problem hiding this comment.
@Parameterized, @CsvSource에 대해서 학습해보고
중복되는 코드를 줄이고 가독성 올려봅시다!
There was a problem hiding this comment.
오 이러한 것들이 있군요. 알려주셔서 감사합니다. 반영해보겠습니다!
|
|
||
| @Test | ||
| @DisplayName("음수가 포함되면 예외를 발생해야 한다.") | ||
| void should_throw_exception_when_contains_minus() { |
There was a problem hiding this comment.
다른 메서드들과 컨벤션이 맞춰지지 않은 것 같아요
메소드 시그니처 라인과의 개행을 하은이만의 기준으로 통일해봅시다!
|
|
||
| } | ||
| } | ||
|
|
src/main/java/StringCalculator.java
Outdated
|
|
||
| public class StringCalculator { | ||
|
|
||
| public int getSum(String str) { |
src/main/java/StringCalculator.java
Outdated
| } | ||
| int sum = 0; | ||
| String separater = "[,|:]"; | ||
| String numbers = str; |
There was a problem hiding this comment.
매개변수로 넘어온 str을 어떤 의도로 numbers라는 변수로 대입했나요?
아래 코드를 쭉 살펴보았을때도 의문만 늘어나는 것 같은데 하은이의 의도를 알려주세요
There was a problem hiding this comment.
현재 코드를 보면, numbers 는 마지막 커스텀 구분자인 \n 이후의 값들을 뜻합니다. 만약 numbers 를 "" 으로 두면,
String[] tokens = numbers.split(separater);
if(tokens.length == 1) {
return 0;
}기본 구분자인 경우에는 올바르게 사용하지 못하기에 numbers에 str 을 대입하였습니다.
하지만, 여전히 헷갈리는 부분이 존재하여 알려주신 피드백 반영하여 새로 코드를 작성해보았습니다!
- should_throw_exception_when_another_wrong_input 이 경우 테스트를 통과하지 못했습니다. 차근차근 다시 해보겠습니다.
문자열 계산기에 숫자 이외의 값 또는 음수를 전달하는 경우 RuntimeException 예외를 throw한다. 는 기능 요구사항과 일치하는지 잘 모르겠습니다.
현재는, "//;1;2;\n123;" 입력 시에, 123이 커스텀 구분자로 구분되지 않으니 "잘못된 입력 형식입니다." 런타임 예외를 발생하는 코드로 작성하였습니다.
하지만, "123"의 경우에는, 구분자가 없으므로 0으로 반환하는 코드로 작성했습니다.
위 두 가지 경우는 어떻게 처리하는 것이 가장 기능 요구사항과 적합하는지요..!