[LBP] 송하은 미션 3,4단계 제출합니다. by haeunsong · Pull Request #72 · next-step/java-calculator-unit-playground

haeunsong · 2025-02-08T09:08:58Z

문자열 계산기에 숫자 이외의 값 또는 음수를 전달하는 경우 RuntimeException 예외를 throw한다. 는 기능 요구사항과 일치하는지 잘 모르겠습니다.

현재는, "//;1;2;\n123;" 입력 시에, 123이 커스텀 구분자로 구분되지 않으니 "잘못된 입력 형식입니다." 런타임 예외를 발생하는 코드로 작성하였습니다.
하지만, "123"의 경우에는, 구분자가 없으므로 0으로 반환하는 코드로 작성했습니다.

위 두 가지 경우는 어떻게 처리하는 것이 가장 기능 요구사항과 적합하는지요..!

woogym

3,4주차 미션 고생하셨어요 😁😁

미션을 잘 수행하신 것 같아요 고민이 많이하신 것 같은 흔적이 보여서 좋았습니다!
특히 테스트 코드를 잘 작성해두어서 이해하기 쉬웠어요
이번주도 하은이만의 답을 찾아봅시다! 리뷰 확인하고 개선해봅시다!

질문에 대한 답변

현재는, "//;1;2;\n123;" 입력 시에, 123이 커스텀 구분자로 구분되지 않으니 "잘못된 입력 형식입니다." 런타임 예외를 발생하는 코드로 작성하였습니다.
하지만, "123"의 경우에는, 구분자가 없으므로 0으로 반환하는 코드로 작성했습니다.
위 두 가지 경우는 어떻게 처리하는 것이 가장 기능 요구사항과 적합하는지요..!

첫번째의 경우 기능 요구 사항에서 정의된 커스텀 구분자 입력 형식에 어긋나보여요
예외를 던지는 것이 요구사항과 적절해보여요
두번째의 경우 123 -> 0을 반환하는 것이 맞는가? -> 저는 아니라고 생각해요
123이라는 숫자도 유효한 숫자라고 생각합니다 유효한 숫자이니 합산 대상이 된다고 생각해요
이 부분에 있어서 코멘트에도 작성해두었으니 확인해봐요~

woogym · 2025-02-09T13:50:09Z

src/main/java/StringCalculator.java

+            // \n 이후에 최소한 구분자가 한 개 이상은 있어야한다. 구분자가 없다면 배열 길이가 1이된다.
+            if(numbers.isEmpty() || numbers.split(separater).length == 1) {
+                throw new RuntimeException("잘못된 입력 형식입니다.");
+            }


해당 조건문에서 "//;\n2"처럼 한 개의 숫자만 입력된 경우 예외가 발생할 가능성이 있어보여요
"2"는 정상적인 입력으로 볼 수 있어야하죠 다시 개선해봅시다!

"구분자가 하나 이상 포함되어야 한다"는 규칙은 요구사항에 없음

woogym · 2025-02-09T13:58:04Z

src/main/java/StringCalculator.java

+            if (newLineIdx == -1) {
+                throw new RuntimeException("잘못된 입력 형식입니다.");
+            }
+            separater = Pattern.quote(str.substring(2, newLineIdx));


정규식 특수문자를 안전하게 처리하셨네요! 👍

woogym · 2025-02-09T13:58:49Z

src/main/java/StringCalculator.java

+            int newLineIdx = str.indexOf("\n");
+            // /n 을 찾을 수 없거나, \n 이후 값이 없는 경우
+            if (newLineIdx == -1) {
+                throw new RuntimeException("잘못된 입력 형식입니다.");


예외 사항을 정확하게 전달해보면 어떨까요?

woogym · 2025-02-09T14:20:03Z

src/main/java/StringCalculator.java

+
+public class StringCalculator {
+
+    public int getSum(String str) {


하나의 메서드가 너무 많은 책임을 가지고 있어요
코드를 단번에 이해하기에는 어려웠어요 좀 더 분리해볼까요?

입력값 검증

구분자 추출

문자열을 구분자 기준으로 분리

분리된 숫자 문자열 배열을 합산
으로 분리해보았습니다!

woogym · 2025-02-09T14:21:05Z

src/main/java/StringCalculator.java

+    public int parseNumber(String number) {
+        // 숫자가 아니라면 예외 발생
+        try {
+            return Integer.parseInt(number);
+        } catch (NumberFormatException e) {
+            throw new RuntimeException("숫자가 아닌 값이 포함되어 있습니다.");
+        }
+    }


외부에서 호출할 필요없는 메서드는 접근제어자를 통해서 캡슐화를 지켜봅시다!

woogym · 2025-02-09T14:21:49Z

src/test/java/StringCalculatorTest.java

+@DisplayName("문자열 계산기 테스트")
+class StringCalculatorTest {
+
+    StringCalculator stringCalculator = new StringCalculator();


해당 클래스에서만 사용된다면 인스턴스 변수 또한 private로 캡슐화를 지켜봅시다

woogym · 2025-02-09T14:23:10Z

src/test/java/StringCalculatorTest.java

+
+    StringCalculator stringCalculator = new StringCalculator();
+
+    @Nested


@Nested를 통해서 테스트를 그룹화 하셨네요 좋습니다!

woogym · 2025-02-09T14:25:31Z

src/test/java/StringCalculatorTest.java

+        @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("숫자가 아닌 값이 포함되어 있습니다.");
+        }


@Parameterized, @CsvSource에 대해서 학습해보고
중복되는 코드를 줄이고 가독성 올려봅시다!

오 이러한 것들이 있군요. 알려주셔서 감사합니다. 반영해보겠습니다!

woogym · 2025-02-09T14:27:24Z

src/test/java/StringCalculatorTest.java

+
+        @Test
+        @DisplayName("음수가 포함되면 예외를 발생해야 한다.")
+        void should_throw_exception_when_contains_minus() {


다른 메서드들과 컨벤션이 맞춰지지 않은 것 같아요
메소드 시그니처 라인과의 개행을 하은이만의 기준으로 통일해봅시다!

woogym · 2025-02-09T14:28:47Z

src/test/java/StringCalculatorTest.java

+
+        }
+    }
+


불필요한 개행들이 보입니다 제거해볼까요?

woogym · 2025-02-09T19:50:07Z

src/main/java/StringCalculator.java

+
+public class StringCalculator {
+
+    public int getSum(String str) {


좀 더 의미있는 역할과 의도에 맞는 변수로 네이밍 해봅시다

woogym · 2025-02-09T19:50:57Z

src/main/java/StringCalculator.java

+        }
+        int sum = 0;
+        String separater = "[,|:]";
+        String numbers = str;


매개변수로 넘어온 str을 어떤 의도로 numbers라는 변수로 대입했나요?
아래 코드를 쭉 살펴보았을때도 의문만 늘어나는 것 같은데 하은이의 의도를 알려주세요

현재 코드를 보면, numbers 는 마지막 커스텀 구분자인 \n 이후의 값들을 뜻합니다. 만약 numbers 를 "" 으로 두면,

String[] tokens = numbers.split(separater); if(tokens.length == 1) { return 0; }

기본 구분자인 경우에는 올바르게 사용하지 못하기에 numbers에 str 을 대입하였습니다.

하지만, 여전히 헷갈리는 부분이 존재하여 알려주신 피드백 반영하여 새로 코드를 작성해보았습니다!

- should_throw_exception_when_another_wrong_input 이 경우 테스트를 통과하지 못했습니다. 차근차근 다시 해보겠습니다.

haeunsong added 5 commits February 8, 2025 01:13

3,4단계 문자열 계산기 테스트 구현

eb6bedd

refactor : 정적 메서드 -> 인스턴스 메서드로 변경

035d8ad

feat: 미션 1,2단계 + 미션 3,4단계

0afd62f

refactor : 불필요한 주석 제거

972b88c

test : 숫자가 아닌 글자가 포함된 경우 예외 발생 케이스 추가

ce29ca1

woogym reviewed Feb 9, 2025

View reviewed changes

refactor : 코드 리뷰 반영하여 수정

15dad54

- should_throw_exception_when_another_wrong_input 이 경우 테스트를 통과하지 못했습니다. 차근차근 다시 해보겠습니다.


		public class StringCalculator {

		public int getSum(String str) {


		StringCalculator stringCalculator = new StringCalculator();

		@Nested

Conversation

haeunsong commented Feb 8, 2025

Uh oh!

woogym left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants