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

Step2 #5926

Merged
merged 1 commit into from
Mar 14, 2025
Merged

Step2 #5926

merged 1 commit into from
Mar 14, 2025

Conversation

cm0x0x0x0
Copy link

@cm0x0x0x0 cm0x0x0x0 commented Mar 13, 2025

리뷰 요청드립니다~

@cm0x0x0x0 cm0x0x0x0 changed the base branch from master to cm0x0x0x0 March 13, 2025 11:33
Copy link

@mskangg mskangg left a comment

Choose a reason for hiding this comment

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

안녕하세요!
이번 미션 리뷰를 맡은 강민수입니다 🙇

2단계 미션 잘해주셨네요! 👏
몇가지 피드백 남겨드렸는데, 확인하시고 다음 단계 진행하시면서 같이 반영 부탁드립니다. 🙏

그럼 다음 단계 진행해주세요! 🔥

Comment on lines +12 to +17
switch (input.length()) {
case 1:
return processSingleCharacter(input);
default:
return processMultipleCharacter(input);
}
Copy link

Choose a reason for hiding this comment

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

case 가 한개이니, 얼리 리턴 방식이 더 나을것 같기도하네요 🤔

Suggested change
switch (input.length()) {
case 1:
return processSingleCharacter(input);
default:
return processMultipleCharacter(input);
}
if (input.length() == 1) {
return processSingleCharacter(input);
}
return processMultipleCharacter(input);

}

private static int processMultipleCharacter(String input) {
Matcher m = Pattern.compile("//(.)\n(.*)").matcher(input);
Copy link

Choose a reason for hiding this comment

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

Pattern.compile 은 상수화 하는것도 좋을것 같아요!

구현체를 살펴보면 매번 새로운 Pattern 을 생성하기 때문이에요 🙃

    /**
     * Compiles the given regular expression into a pattern.
     *
     * @param  regex
     *         The expression to be compiled
     * @return the given regular expression compiled into a pattern
     * @throws  PatternSyntaxException
     *          If the expression's syntax is invalid
     */
    public static Pattern compile(String regex) {
        return new Pattern(regex, 0);
    }

Comment on lines +40 to +44
@Test
public void splitAndSum_negative() throws Exception {
assertThatThrownBy(() -> StringAddCalculator.splitAndSum("-1,2,3"))
.isInstanceOf(RuntimeException.class);
}
Copy link

Choose a reason for hiding this comment

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

예외메세지도 추가로 검증하는것도 좋을 것 같아요!
그리고 문자열 계산기에 숫자 이외의 값인 경우 RuntimeException 예외를 throw한다. 를 검증하는 테스트코드가 보이지 않네요 😅
TDD 과정이니 요구사항을 검증할 수 있는 테스트코드를 먼저 작성한 뒤 기능을 구현해보시는것 추천드립니다!

@mskangg mskangg merged commit 69d92ab into next-step:cm0x0x0x0 Mar 14, 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.

2 participants