-
Notifications
You must be signed in to change notification settings - Fork 0
[문자열 덧셈 계산기] 양화영 미션 제출합니다 #3
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
base: main
Are you sure you want to change the base?
Conversation
525tea
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
화영님 코드 잘 보았습니다 😊
예외 처리나 테스트 코드가 세분화되어 있어서 정말 꼼꼼히 구현해주셨다는걸 느꼈습니다
출력 메시지를 enum 처리해주신 것도 인상 깊었고, 테스트 코드 부분에서는 많이 배웠습니다.!
좋은 코드 공유해주셔서 감사합니다 🙇♀️
| private static final Pattern CUSTOM_PATTERN = Pattern.compile("//(.*?)\\\\?n(.*)", Pattern.DOTALL); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
커스텀 구분자 처리를 정규식으로 구현해주셨는데요
"\\\\?n" 패턴은 "\n"이 아닌 문자 '\' + 'n'의 조합을 인식하게 될거 같아요.
즉, "//;\\n1;2;3"같은 입력에는 잘 동작하지만 실제 콘솔 입력에서는 \n이 개행으로 처리되기 때문에 이 부분이 커버되지 않을 수 있을 것 같아요
따라서 "//(.*?)\n(.*)"로 실제 개행 문자를 인식하도록 수정하면 어떨까 싶습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 수정 하도록 하겠습니다 . 감사합니다 :)
| //입력값이 null인 경우 0을 반환 | ||
| if (input == null || input.isEmpty()) { | ||
| return "0"; | ||
| } | ||
| //공백만 입력하거나 공백을 문자에 포함한 경우의 예외 처리 | ||
| if (input.trim().isEmpty() || input.contains(" ") ) { | ||
| throw new IllegalArgumentException(ErrorMessage.TRIM_ERROR); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
입력값 검증을 구체적으로 세분화해주셨군요👍
input == null : 입력이 아예 없는 경우 ex. (null)
input.isEmpty() : 빈 문자열(””)인 경우 ex. ""
input.trim().isEmpty() : 공백만 입력한 경우 ex. " ", "\t", "\n"
input.contains(" ") : 문자열 내부에 공백이 포함된 경우 ex. "1 2", " 1,2"
이렇게 각각의 케이스로 나누어 검증하신 것 같아요 맞을까요?
만약 그렇다면 제 생각에는 input.trim().isEmpty()가 input.isEmpty()를 포함해 커버할 것 같은데요,
따라서 input.trim().isEmpty() 하나만 남겨도 될 것 같은데 어떻게 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trim().isEmpty()는 " "만 잡고, 빈 문자열 ""을 못잡기에 input.isEmpty() 가 없으면 NPE 가 발생할 수도 있을 것 같아서 둘다 남겨뒀습니다 !
| for (Integer number : numbers) { | ||
| sum += number; | ||
| } | ||
| System.out.println(SuccessMessage.OUTPUT_MESSAGE + " " + sum); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
출력 메시지를 enum 처리해준 부분 정말 좋은거 같아요!
이렇게 하면 코드 재사용성과 유지보수성이 높아질 것 같아요
좋은 인사이트 감사합니다 👍
| } | ||
|
|
||
| @DisplayName("커스텀 구분자 적용") | ||
| @ParameterizedTest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트 코드를 구조화하신 부분이 정말 좋은 것 같아요 👍👍
각 테스트가 무엇을 검증하는지 명확하게 드러나서 가독성이 높은거 같아요
@ParameterizedTest로 다양한 케이스를 커버하고, @ValueSource로 입력값을 깔끔하게 관리하는 방식을 사용하셨군요. JUnit 기능 활용 한 수 배워갑니다 🙇♀️
No description provided.