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

2단계 - 문자열 덧셈 계산기 #5921

Merged
merged 2 commits into from
Mar 13, 2025

Conversation

sundongkim-dev
Copy link

No description provided.

Copy link

@wooobo wooobo left a comment

Choose a reason for hiding this comment

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

2단계 잘진행해 주셨네요!
몇가지 코멘트 남겨 놓았어요!

2단계 merge진행하겠습니다.
본격 자동차 경주 시작해보시죠~

화이팅입니다!

if (!INTEGER_PATTERN.matcher(str).matches()) {
throw new RuntimeException("숫자만 입력가능하지만 입력받은 문자열은 " + str + "입니다!");
}
if (Integer.parseInt(str) <= 0) {
Copy link

Choose a reason for hiding this comment

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

Number같은 클래스가 있다면 해당 조건들을 지켜주는 온전한 객체를 만들어 볼수있겠네요!

return result;
}

public static int splitAndSum(String input) {
Copy link

Choose a reason for hiding this comment

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

한 메서드는 한 가지 일만 해야 한다.

다소 많은 기능으르 수행하고있는데 더 작은 단위로 분리할수 없을지 고민해보면 좋겠네요!

Comment on lines +10 to +14
int result = StringAddCalculator.splitAndSum(null);
assertThat(result).isEqualTo(0);

result = StringAddCalculator.splitAndSum("");
assertThat(result).isEqualTo(0);
Copy link

Choose a reason for hiding this comment

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

Suggested change
int result = StringAddCalculator.splitAndSum(null);
assertThat(result).isEqualTo(0);
result = StringAddCalculator.splitAndSum("");
assertThat(result).isEqualTo(0);
assertThat(StringAddCalculator.splitAndSum(null)).isEqualTo(0);
assertThat(StringAddCalculator.splitAndSum("")).isEqualTo(0);
  • 변수명을 재활용하면 추후 유지보수할때 혼란을 야기시킬수 있는 포인트가 될수도 있다고 생각합니다~
  • 좀 더 명확한 테스트 코드를 고민해보아요~
  • assertAll을 사용해도 좋겠네요~
  • @NullAndEmptySource 활용해볼수도 있겠네요!

@@ -0,0 +1,58 @@
package calculator;
Copy link

Choose a reason for hiding this comment

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

@wooobo wooobo merged commit 91a57fa into next-step:sundongkim-dev Mar 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