[LBP] 윤준석 3, 4단계 과제 제출합니다. #74

junseok0304 · 2025-02-09T06:00:05Z

스터디 정보

스터디명: LBP
리뷰이: 윤준석
리뷰어: 이한길

질문

코드 개행이나 가독성 관련하여 개선할 점이 있다면 알려주시면 감사하겠습니다.
43번째 줄에 throw new RuntimeException("Invalid input: 숫자가 아닌 값이 포함되었습니다."); 과 같이
"영문 오류 문구 : 한글 세부내용" 형식을 차용해보았는데, 오류 발생시에 가독성과 사용성이 좋았습니다.
위 경우처럼 영문 오류 문구와 한글 세부내용을 혼용하여 사용하는 방식도 권장되는 방식인건지 궁금합니다!
구분자(구분 기호)에 대한 변수명을 delimiter 로 설정했는데 직관적인지 아니면 조금 모호한 느낌이 드는지 궁금합니다.
테스트 코드상의 DisplayName이 적절한지 궁금합니다! (어떤 기능을 하는 코드인지를 표출하는 이름)

이유

이번 과제 해결을 위해 정규식 문자열 처리 관련 메서드를 찾아보던 중에 Pattern.quote를
사용하여 특수문자가 포함된 문자열을 안전하게 처리하는 방법을 찾아서 적용해보았습니다!
지난번과 다르게, assertJ를 활용해 테스트 코드를 작성한 이번 케이스에서는 @nested를 사용하지 않았습니다.
(@nested를 사용하면 불필요하게 코드가 복잡해질 수 있다고 생각했습니다!)

< 구현에 사용했던 사전 조건 >

조건 1. 쉼표(,) 또는 콜론(:)을 구분자로 가지는 문자열을 전달하는 경우 구분자를 기준으로 분리한 각 숫자의 합을 반환
(예: “” => 0, "1,2" => 3, "1,2,3" => 6, “1,2:3” => 6)
조건 2. 앞의 기본 구분자(쉼표, 콜론)외에 커스텀 구분자를 지정할 수 있다.
커스텀 구분자는 문자열 앞부분의 “//”와 “\n” 사이에 위치하는 문자를 커스텀 구분자로 사용한다.
예를 들어 “//;\n1;2;3”과 같이 값을 입력할 경우 커스텀 구분자는 세미콜론(;)이며, 결과 값은 6이 반환되어야 한다.
조건 3. 문자열 계산기에 숫자 이외의 값 또는 음수를 전달하는 경우 RuntimeException 예외를 throw한다.

추가로 기본 구분자 및 커스텀 구분자가 혼용되는 경우에 대해서 처리할 수 있도록 코드 작성.

테스트 실패 시 출력할 메시지이나, 성공으로 작성된 문구를 수정하였습니다.

🎉 PR 머지 완료! 🎉

불필요한 개행 제거

hangillee

안녕하세요, 준석! 👋
3, 4단계 미션은 문자열을 다뤄야 해서 조금 까다로웠을 것 같아요.

고민한 흔적도 잘 보이고, 스스로 근거를 세워서 미션 잘 해결하셨습니다! 제가 보기엔 준석이 작성한 변수명이나 @DisplayName의 내용들은 크게 문제 없어 보입니다. 명확하고 직관적이어서 이해하기 쉬웠습니다.

앞으로 계속해서 프로젝트와 협업을 하게 될텐데 왜 이런 스타일로 작성했는지 이유만 확실하게 있으면 된다고 생각합니다. 초록 스터디의 제1목표는 나만의 코드 스타일 정립이니까요. 최소한 정답은 없더라도 오답은 있기 때문에, 나만의 정답을 계속 세워나가 봅시다! 😁

제가 하고 싶은 말은 코멘트로 달아두었습니다. 확인해보시고 반영해주신 다음 디스코드 채널 멘션을 통해 리뷰 요청해주세요! 파이팅입니다!

hangillee · 2025-02-09T08:52:14Z

src/main/java/StringCalculator.java

+import java.util.regex.Pattern;
+
+
+/**


무의미한 공백인 것 같아 보입니다! 한 줄 제거해볼까요?

hangillee · 2025-02-09T08:53:32Z

src/main/java/StringCalculator.java

+ * 문자열을 숫자로 변환하여 합산하는 계산기임. <br>
+ * 쉼표 또는 콜론을 기본 구분자로 사용함.<br>
+ * //<구분자>\n 해당 위치에 원하는 구분자를 넣으면,<br>
+ * 단일 문자로 커스텀 구분자 지정이 가능함.<br>
+ * //@\n1:2@3,4 같은 형식도 처리 가능<br>
+ * 음수나 숫자가 아닌 값이 포함되면 예외를 발생시킴.
+ */


이 주석은 더 이상 필요 없을 것 같은데 어떻게 생각하시나요?
만약 현재 코드가 프로덕션 배포된 코드라면 이 주석이 문서로서 제 역할을 다 할 수 있을까요?

만약 StringCalculator.java 소스코드에 대한 Docs를 작성하고자 한다면 참고할만 한 아티클이 있을까요?

API 문서화 툴은 정말 다양하지만 순수 Java 어플리케이션에서는 Javadoc이 떠오르네요.

한번 찾아보겠습니다!

hangillee · 2025-02-09T08:55:04Z

src/main/java/StringCalculator.java

+public class StringCalculator {
+
+    public static int add(String input) {


여기서는 클래스 시그니처와 첫 번째 메소드 시그니처 사이에 개행을 두셨네요.
이 스타일도 많이 사용되는 스타일입니다. (실제로 저는 팀 프로젝트 때 이 형식으로 컨벤션을 정했습니다.)

다만, 준석이 페어와 함께 작성한 코드인 Calculator 클래스에서는 이러한 형식이 아닌 것으로 보입니다.
페어와 함께한 코드도 준석의 스타일대로 리팩토링 해보는 건 어떨까요? 어느 쪽이 준석의 스타일인가요?

현재와 같이 구분해서 띄우는 방법이 가독성이 좋은 것 같아서 앞으로는 이 방법으로 통일하겠습니다!

hangillee · 2025-02-09T09:01:29Z

src/main/java/StringCalculator.java

+        String delimiter = "[,|:]"; // 기본 구분자 정의
+
+        if (input.startsWith("//")) { // 커스텀 구분자 처리 (여러 글자 지원)
+            int delimiterEnd = input.indexOf("\n");
+            String customDelimiter = input.substring(2, delimiterEnd);
+            delimiter = "[,|:]|" + Pattern.quote(customDelimiter);
+            input = input.substring(delimiterEnd + 1);
+        }


여기 들어가는 매직 넘버, 매직 리터럴 친구들 모두 상수로 만들어주면 어떨까요?

나중에 계산기의 커스텀 구분자 기준이 달라지지 않을 거란 보장이 있을까요?
매직 넘버, 매직 리터럴을 사용했을 때의 문제는 무엇이 있을까요?

일단 저는 한번에 이해하기 어려웠던 것 같습니다. 😁

매직 넘버나 매직 리터럴은 변경사항이 발생할 경우 일일히 수정해줘야한다는 단점이 있는 것 같습니다.
또한 재사용이 불가하므로 상수로 정의해서 사용하거나 Enum을 사용하는 방법도 있을 것 같습니다!

// 상수 정의
private static final String DEFAULT_DELIMITER = "[,|:]";
private static final String CUSTOM_DELIMITER_PREFIX = "//";
private static final String NEW_LINE = "\n";
private static final int CUSTOM_DELIMITER_PREFIX_LENGTH = 2;

src/main/java/StringCalculator.java

hangillee · 2025-02-09T09:32:39Z

src/main/java/StringCalculator.java

+            try {
+                value = Integer.parseInt(num); // parseInt 사용하여 문자열을 숫자로 변환
+            } catch (NumberFormatException e) {
+                // 숫자가 아닌 값이 포함되면 런타임 에러 발생
+                throw new RuntimeException("Invalid input: 숫자가 아닌 값이 포함되었습니다.");
+            }
+            // 음수값이 발견되면 negativeNumber에 추가
+            if (value < 0) {
+                negativeNumber.append(value).append(" ");
+            }


이 부분을 메소드로 분리해서 들여쓰기 단계를 줄여봅시다!
현재 sum 메소드가 꽤 긴 것 같아서 가독성이 조금 떨어지는 것 같아요.

private int parseAndValidateNumber(String num, StringBuilder negativeNumbers) { int value; // 문자열을 숫자로 변환 try { value = Integer.parseInt(num); } catch (NumberFormatException e) { throw new RuntimeException("Invalid input: 숫자가 아닌 값이 포함되었습니다."); } if (value < 0) { negativeNumbers.append(value).append(" "); } return value; }

SRP를 지키면서 가독성을 확보하는 방법이 있군요

hangillee · 2025-02-09T09:34:09Z

src/test/java/StringCalculatorTest.java

+    @Test
+    @DisplayName("단일 숫자 출력 기능")
+    void testSingleNumber() {
+        assertThat(StringCalculator.add("1")).isEqualTo(1);
+        assertThat(StringCalculator.add("5")).isEqualTo(5);
+    }


@ParameterizedTest에 대해서 알고 있나요?
학습해보고 반복되고 있는 코드를 제거해봅시다!

Junit5에서 제공하는 @CsvSource를 활용해주면 assertThat을 과다하게 사용하지 않아도 되서 편할 것 같네요!

hangillee · 2025-02-09T09:37:52Z

src/test/java/StringCalculatorTest.java

+    void testNegativeNumbersThrowException() {
+        assertThatThrownBy(() -> StringCalculator.add("1,-2,3"))
+                .isInstanceOf(RuntimeException.class)
+                .hasMessageContaining("입력한 값 중 음수 값이 존재합니다.");


예외 메시지가 제대로 출력되고 있는지 파악하려면 구현한 그대로 나오는지 테스트해야 합니다.
즉, hasMessageContaining()이 아닌 hasMessage()를 활용해 일치하는지 확인해야 합니다.

일치가 아닌 포함으로 검증하게 되면 준석이 구현한 대로, 입력된 음수를 예외 메시지에 제대로 포함시키는지,
아니면 그렇지 않은지 테스트만으로는 알 수 없게 됩니다. 관련해서 Fail-Fast에 대해 알아보면 좋을 것 같아요.

중요한 부분을 놓친 것 같습니다! 감사합니다.

문제가 발생하면 즉시 예외처리 Fail-Fast

문제가 발생해도 작업을 중단하지 않고 진행 Fail-Safe

hangillee · 2025-02-09T09:39:37Z

src/main/java/StringCalculator.java

+                value = Integer.parseInt(num); // parseInt 사용하여 문자열을 숫자로 변환
+            } catch (NumberFormatException e) {
+                // 숫자가 아닌 값이 포함되면 런타임 에러 발생
+                throw new RuntimeException("Invalid input: 숫자가 아닌 값이 포함되었습니다.");


예외 메시지 역시 코드 컨벤션의 영역이라 뭐가 옳고 그르다라고 말할 수 없을 것 같습니다.
준석이 생각하기에 이 형식이 보기 쉽고 이해하기 쉽다면 상관 없을 것 같습니다.

감사합니다! 리뷰해주신대로 수정해보겠습니다.

Co-authored-by: Lee Hangil <[email protected]>

hangillee

안녕하세요, 준석! 👋
리뷰 반영하느라 고생하셨습니다.

코멘트 확인해주시고 디스코드 채널에서 멘션 주시면 다음엔 머지하도록 할게요!

p.s. 다음 미션부턴 코드를 수정하고 난 후, PR 화면에서 오른쪽 상단에 Reviewers 부분에서 RC를 요청할 수 있습니다!

hangillee · 2025-02-10T11:05:03Z

src/main/java/StringCalculator.java

+        // 음수값이 존재하면 예외 발생
+        if (!negativeNumbers.isEmpty()) {
+            throw new RuntimeException("입력한 값 중 음수 값이 존재합니다: " + negativeNumbers.toString().trim());
        }


생각해보니 어차피 예외를 던질 코드라면 입력된 모든 음수를 파악해서 알리는 것보다
아래 parseAndValidateNumber()의 조건문에서 바로 예외를 던지는 편이 더 낫지 않을까요?

준석의 생각은 어떤가요?

반영 완료했습니다!

hangillee · 2025-02-10T11:05:20Z

src/test/java/StringCalculatorTest.java

+    @ParameterizedTest
+    @DisplayName("단일 숫자 입력 테스트")
+    @CsvSource({
+            "1, 1",
+            "5, 5",
+            "10, 10"
+    })
+    void testSingleNumber(String input, int expected) {
+        assertThat(StringCalculator.add(input)).isEqualTo(expected);
    }


hangillee · 2025-02-10T11:06:12Z

src/test/java/StringCalculatorTest.java

    @Test
-    @DisplayName("입력값이 숫자가 아닌 값이 존재하는 경우를 식별하는 기능")
+    @DisplayName("입력값이 숫자가 아닌 값이 존재하는 경우 예외 발생")
    void testNonNumericValuesThrowException() {
        assertThatThrownBy(() -> StringCalculator.add("1,a,3"))
-                .isInstanceOf(RuntimeException.class);
+                .isInstanceOf(RuntimeException.class)
+                .hasMessage("Invalid input: 숫자가 아닌 값이 포함되었습니다.");

        assertThatThrownBy(() -> StringCalculator.add("//;\n1;B;3"))
-                .isInstanceOf(RuntimeException.class);
+                .isInstanceOf(RuntimeException.class)
+                .hasMessage("Invalid input: 숫자가 아닌 값이 포함되었습니다.");
    }


이 두 친구도 @ParameterizedTest를 활용할 수 있을 것 같은데 어떤가요?

반영 완료!

(입력한 값 중 음수 값이 존재시 조건문에서 바로 예외처리), 테스트 코드에서 @ParameterizedTest 활용 가능한 부분 추가 수정

junseok0304 and others added 6 commits February 4, 2025 20:57

1,2단계 미션 완료(윤준석)

30862c7

Fix: assertEquals, assertThrows의 세 번째 인자는

5ea3434

테스트 실패 시 출력할 메시지이나, 성공으로 작성된 문구를 수정하였습니다.

Fix: 개행을 수정하였습니다.

cbba6ea

고생하셨습니다.

29234dd

🎉 PR 머지 완료! 🎉

feat: 3,4단계 문자열 계산기 및 assertJ를 활용한 테스트 코드 작성 및 테스트 미션 수행

0914fac

Update build.gradle

11e3ccf

불필요한 개행 제거

hangillee suggested changes Feb 9, 2025

View reviewed changes

junseok0304 and others added 4 commits February 10, 2025 17:43

Update src/main/java/StringCalculator.java

a489324

Co-authored-by: Lee Hangil <[email protected]>

feat: 리뷰 받은 사항을 반영하고 코드를 최적화 했습니다.

27d5b9e

Merge remote-tracking branch 'origin/junseok0304' into junseok0304

d13d70a

Merge branch 'next-step:junseok0304' into junseok0304

bbc05d2

hangillee approved these changes Feb 10, 2025

View reviewed changes

junseok0304 added 2 commits February 11, 2025 21:56

feat: 리뷰 받은 사항을 반영하였습니다.

cf683c9

(입력한 값 중 음수 값이 존재시 조건문에서 바로 예외처리), 테스트 코드에서 @ParameterizedTest 활용 가능한 부분 추가 수정

Merge remote-tracking branch 'origin/junseok0304' into junseok0304

d20e9a1

junseok0304 requested a review from hangillee February 11, 2025 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LBP] 윤준석 3, 4단계 과제 제출합니다. #74

[LBP] 윤준석 3, 4단계 과제 제출합니다. #74

junseok0304 commented Feb 9, 2025

hangillee left a comment

hangillee Feb 9, 2025

hangillee Feb 9, 2025

junseok0304 Feb 10, 2025

hangillee Feb 10, 2025

junseok0304 Feb 11, 2025

hangillee Feb 9, 2025

junseok0304 Feb 10, 2025

hangillee Feb 9, 2025

junseok0304 Feb 10, 2025

hangillee Feb 9, 2025

junseok0304 Feb 10, 2025 •

edited

Loading

hangillee Feb 9, 2025

junseok0304 Feb 10, 2025

hangillee Feb 9, 2025

junseok0304 Feb 10, 2025 •

edited

Loading

hangillee Feb 9, 2025

junseok0304 Feb 10, 2025

hangillee left a comment •

edited

Loading

hangillee Feb 10, 2025

junseok0304 Feb 10, 2025 •

edited

Loading

hangillee Feb 10, 2025

hangillee Feb 10, 2025

junseok0304 Feb 11, 2025 •

edited

Loading

		public class StringCalculator {

		public static int add(String input) {

		import java.util.regex.Pattern;


		/**

[LBP] 윤준석 3, 4단계 과제 제출합니다. #74

Are you sure you want to change the base?

[LBP] 윤준석 3, 4단계 과제 제출합니다. #74

Conversation

junseok0304 commented Feb 9, 2025

스터디 정보

질문

이유

< 구현에 사용했던 사전 조건 >

hangillee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junseok0304 Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junseok0304 Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hangillee left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junseok0304 Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junseok0304 Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

junseok0304 Feb 10, 2025 •

edited

Loading

junseok0304 Feb 10, 2025 •

edited

Loading

hangillee left a comment •

edited

Loading

junseok0304 Feb 10, 2025 •

edited

Loading

junseok0304 Feb 11, 2025 •

edited

Loading