-
Notifications
You must be signed in to change notification settings - Fork 0
[문자열 덧셈 계산기] 김희영 미션 제출합니다. #6
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
sanchaehwa
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.
1주차 고생 많으셨습니다!
클래스 분리가 잘 되어 있어서 각 클래스의 역할과 전체 구조를 이해하는 데 많은 도움이 되었습니다.
몇 가지 개선이 가능해 보이는 부분에 대해서는 코멘트로 제안사항을 남겨놨습니다 🙂
|
|
||
| for (String token : tokens) { | ||
| validator.validateNumber(token); | ||
| numbers.add(Integer.parseInt(token)); |
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.
validateNumber() 내부에서도 Integer.parseInt(token)을 수행하고 있기 때문에 검증된 int 값을 그대로 반환받아 사용하는 것도 좋을 것 같아요
그래서 validateNumber()에서 검증이 완료된 값을 int로 반환하도록 수정하면 어떨까 싶습니다 !
|
|
||
| public void validateInput(String input) { | ||
| if (input == null) { | ||
| throw new IllegalArgumentException("입력값이 null입니다."); |
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만 검증하고 있는데, 빈 문자열("")이 들어온 상태에서 Integer.parseInt()를 호출하면 NumberFormatException이 발생할 수 있을 것 같아요.
그래서 input.trim().isEmpty() 조건을 함께 추가해서 빈 문자열이나 공백만 있는 입력값에 대한 예외처리를 해주면 좋을 것 같습니다 !
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.
@Huiyeongkim 코드 잘 보았습니다 😊
클래스 책임 분리가 잘 되어 있어서 코드의 흐름을 읽기 쉬웠습니다.
1주차 고생 많으셨습니다 🔥
| if (input.isEmpty()) { | ||
| return new ArrayList<>(); | ||
| } |
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.isEmpty()로 빈 문자열은 방어되지만 " "(공백), "1, 2"(내부에 공백 포함) 등의 케이스는 방어가 안될 것 같습니다.
따라서 입력값 전처리로 .trim() 후 파싱하거나 .contains(" ")로 검증하는 등의 검증 로직을 더해주시면 더 안전할 것 같아요
| int number = Integer.parseInt(token); | ||
| if (number < 0) { | ||
| throw new IllegalArgumentException("음수는 입력할 수 없습니다."); | ||
| } |
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.
validateNumber()에서 숫자 검증과 파싱이 한번에 수행되고 있는데
검증만 담당하거나, parseAndValidate(String) : int 형태로 int 반환까지 책임지는 방식 중 하나로 역할을 명확히 분리하면 좋을 것 같습니다
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException("숫자 형식이 올바르지 않습니다."); | ||
| } |
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.
입력 관련 오류만 따로 구분하기 위해 InvalidInputException 같은 커스텀 예외를 사용하는 것도 고려해보는 것도 좋을 것 같습니다
이렇게 하면 로그 레벨이나 에러 메시지 규격을 일관되게 관리하기가 더 쉬워질 것 같아요!
No description provided.