Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LBP] 박세은 3, 4단계 과제 제출합니다. #75

Merged
merged 10 commits into from
Feb 13, 2025

Conversation

seun0123
Copy link

@seun0123 seun0123 commented Feb 9, 2025

Reviewer : 이지윤
Pair : 권지후


요구사항 체크리스트

3단계 - 문자열 계산기 구현

  • 쉼표(,) 또는 콜론(:)을 구분자로 가지는 문자열을 전달하는 경우 구분자를 기준으로 분리한 각 숫자의 합을 반환
  • 커스텀 구분자는 문자열 앞부분의 “//”와 “\n” 사이에 위치하는 문자를 커스텀 구분자로 사용한다.
  • 문자열 계산기에 숫자 이외의 값 또는 음수를 전달하는 경우 RuntimeException 예외를 throw한다.

4단계 - 리팩토링

  • 기존 JUnit5로 작성되어 있던 단위 테스트를 AssertJ로 리팩터링한다.
  • 메인 메서드는 만들지 않는다.

구현방식

1-2단계 코드 리뷰에서 받은 메서드 네이밍 팁과 코딩 컨벤션 단축키를 활용하여 구현 및 리팩토링을 진행하였습니다!

3단계 - 문자열 계산기 구현

  • ExpressionToTokens(String expression) : 문자열을 숫자로 변환
    • 기본 구분자 , 또는 :로 구분된 숫자를 분리
    • 커스텀 구분자(//[구분자]\n)가 포함된 경우 해당 구분자로 분리
      • extractCustomDelimiter(String expression) : //와 \n 사이의 커스텀 구분자를 추출하여 반환
    • extractToken(String expression) : 문자열에서 //[구분자]\n 부분을 제거하여 연산에 사용할 숫자만 남김
    • 숫자 외의 값이 포함되면 RuntimeException을 발생
  • 사칙연산 (합, 차, 곱, 몫)
    • add(String expression) 메서드 : 문자열을 파싱한 후 모든 숫자를 더함
    • subtract(String expression): 문자열을 파싱한 후 첫 번째 숫자에서 나머지를 뺌
    • multiply(String expression): 문자열을 파싱한 후 모든 숫자를 곱함
    • divide(String expression): 문자열을 파싱한 후 첫 번째 숫자를 기준으로 나눔
      • 나누는 값이 0이면 ArithmeticException을 발생

4단계 - 리팩토링

  • 기존 테스트 코드 : JUnit을 활용하여 assertEquals와 assertThrows를 활용하여 테스트코드 작성
  • 리팩토링 테스트 코드 : AssertJ를 활용하여 assertThat().isEqualTo()와 assertThatThrownBy을 활용하여 테스트코드 작성

질문

  1. 문자열 조작 메서드(substring, indexOf)와 스트림을 활용하여 간결한 코드를 작성하고자 했는데 더 보완할 부분이 있는지 궁금합니다.
  2. private 메서드(ExpressionToTokens, extractCustomDelimiter, extractToken, StringArrayToIntArray)를 세분화하여
    각 기능을 독립적으로 구현했는데, 더 나은 방법이 있는지 궁금합니다.
  3. private 메서드는 직접 테스트할 수 없는데, 사칙연산 메서드 외에도 테스트가 필요할까요?
    만약 필요하다면, private 메서드를 public으로 변경하는 것이 적절한 방법인지 궁금합니다.
  4. 테스트 코드가 충분한지 점검하고 싶습니다. 현재 테스트 코드에서 추가해야 할 케이스가 있을까요?
  5. 메서드 네이밍과 코딩 컨벤션이 적절하게 이루어졌는지 궁금합니다.

Copy link

@dd-jiyun dd-jiyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 세은👋

이번 미션도 고생하셨습니다! 지난 리뷰에 대해서 리팩토링 열심히 해주셨네요 👍🏻
메서드 네이밍과 코드 컨벤션도 많이 고민하신 것 같아요 🤭

이번 미션 요구 사항에 맞춰서 관련된 코드를 위주로 봤습니다!
세은님이 주신 질문 중 일부는 코드 리뷰로 적어놨는데 혹시 더 궁금하신 사항이 있으시면 질문해주세요!


private 메서드는 직접 테스트할 수 없는데, 사칙연산 메서드 외에도 테스트가 필요할까요?
만약 필요하다면, private 메서드를 public으로 변경하는 것이 적절한 방법인지 궁금합니다.

현재 private 메서드는 static으로 정의되어 있는데, 그 이유가 있으신가요?
private 메서드는 말씀하신 대로 클래스 외부에서 직접 호출할 수 없기 때문에, 단위 테스트를 진행하는 데 어려움이 있습니다. 만약 private 메서드의 동작까지 테스트하고 싶으시다면, 해당 메서드를 public으로 변경하는 방법도 있을 수 있습니다.

하지만 일반적으로 private 메서드는 클래스 내부의 구현 세부사항으로, 외부에서 직접 테스트할 필요는 없다고 생각합니다.

따라서, 내부 구현에 대한 테스트가 꼭 필요하지 않다면, private 메서드를 그대로 두고, public 메서드의 동작을 테스트하는 방식으로 진행하는 것이 더 적합할 것 같아요!

build.gradle Show resolved Hide resolved
src/main/java/StringCalculator.java Outdated Show resolved Hide resolved
src/test/java/CalculatorTest.java Show resolved Hide resolved
src/main/java/StringCalculator.java Outdated Show resolved Hide resolved
src/main/java/StringCalculator.java Outdated Show resolved Hide resolved
src/main/java/StringCalculator.java Outdated Show resolved Hide resolved
src/main/java/StringCalculator.java Outdated Show resolved Hide resolved
src/test/java/StringCalculatorTest.java Outdated Show resolved Hide resolved
src/test/java/StringCalculatorTest.java Outdated Show resolved Hide resolved
src/test/java/StringCalculatorTest.java Show resolved Hide resolved
@seun0123
Copy link
Author

seun0123 commented Feb 11, 2025

현재 private 메서드는 static으로 정의되어 있는데, 그 이유가 있으신가요?

A. static을 사용한 메서드들을 모두 입력값을 받아 처리한 후 결과를 반환하는 책임만을 갖고 있는 함수라고 생각해서 객체를 생성하지 않고도 클래스 차원에서 직접 호출이 가능하도록 static으로 정의하였습니다.

제가 궁금했던 부분들에 대해 성심껏 답변해주셔서 감사합니다.
또한, Pattern 등 추가로 제안해주신 공부해야 할 부분들은 제게 많은 도움이 될 것 같습니다.
조언해주신 내용을 바탕으로 다시 리팩토링을 진행한 후, 다시 요청드리겠습니다! 😊

@seun0123
Copy link
Author

지윤님이 주신 코드 리뷰를 반영하여 리팩토링한 후 다시 푸시하였습니다!

변경사항

  1. 사칙연산이 아닌 문자열의 합만 구현하도록 변경
  2. 문자열의 합을 검증하는 모든 테스트 케이스의 actual 값을 숫자로 지정하여 일관성 향상
  3. Java 네이밍 컨벤션을 따라 모든 메서드명을 소문자로 시작하도록 변경
  4. extractCustomDelimiter메서드와 extractToken 메서드에서 정규표현식 //(.*?)\n을 활용하여 커스텀 구분자를 추출하도록 변경
  5. Pattern을 공부하여 extractCustomDelimiter 메서드와 extractToken메서드에 //(.*?)\n를 활용하여 커스텀 구분자를 추출하는 방식으로 수정
    • matcher.group(1)을 사용하여 //와 개행 문자 사이의 내용을 그대로 가져오도록 함
    • Pattern.quote(matcher.group(1))을 적용하여 특수문자가 포함된 구분자도 안전하게 처리할 수 있도록 함
  6. expression.replaceFirst를 활용하여 문자열에서 정규표현식과 일치하는 첫 번째 항목만 치환하도록 함
  7. 문자열의 합, 커스텀 구분자가 포함된 문자열의 합, 예외 상황 등을 별도의 그룹으로 테스트 코드 작성
  8. 조언해주신 예외 상황 포함하여 테스트 작성
  9. 각 테스트 메서드의 메서드명과 @DisplayName을 좀 더 직관적이고 자세한 명칭으로 변경

Pattern을 공부할 때 참고한 블로그
Pattern.quote를 이해할 때 참고한 글

사소한 질문

  • if 문을 Stream을 활용하여 간결하게 작성하는 것이 더 효율적인 코드일까요? 아니면 개발자가 자유롭게 선택하는 부분인가요?
    개인적으로는 아직까지 if 문과 for 문이 더 익숙하지만, Stream을 의식적으로 활용하며 공부하는 것이 맞을지 궁금합니다!

@seun0123 seun0123 requested a review from dd-jiyun February 12, 2025 16:48
Copy link

@dd-jiyun dd-jiyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변경해야할 것이 많았을 텐데 열심히 해주셔서 감사합니다 👍🏻
고생많으셨어요!

다음 미션도 화이팅 🔥


if 문을 Stream을 활용하여 간결하게 작성하는 것이 더 효율적인 코드일까요? 아니면 개발자가 자유롭게 선택하는 부분인가요?
개인적으로는 아직까지 if 문과 for 문이 더 익숙하지만, Stream을 의식적으로 활용하며 공부하는 것이 맞을지 궁금합니다!

정답은 없다고 생각해요!
if / for문과 Stream을 사용하는 것의 차이를 알고 있으면, 상황에 따라 더 적절한 방법을 선택할 수 있겠죠?
또한, Stream을 학습해서 나쁠 건 없으니, 익숙해질 때까지 의식적으로 활용해보는 것도 좋은 방법이라고 생각합니다!

@boorownie boorownie merged commit b56603a into next-step:seun0123 Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants