-
Notifications
You must be signed in to change notification settings - Fork 227
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
1단계 - 문자열 계산기 구현 #836
base: younghoonkwon
Are you sure you want to change the base?
1단계 - 문자열 계산기 구현 #836
Conversation
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.
안녕하세요.
이번 step 잘 진행해 주셨습니다! 👍
몇 가지 코멘트 남겼습니다. 확인 부탁드려요.
궁금하거나 함께 고민이 필요한 부분이 있으시면 코멘트나 DM 남겨주시면 좋을 것 같습니다!
|
||
public class Delimiters { | ||
private final Set<String> delimiters; |
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.
Delimiters 일급컬렉션 👍
Delimiter 자체도 포장해 볼 수 있지 않을까요?
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.
테스트도 필요하지 않을까요?
public String[] split(String text) { | ||
for (String delimiter : delimiters) { | ||
text = text.replace(delimiter, " "); | ||
} | ||
return text.split(" "); | ||
} |
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.
parameter 값을 재할당해 사용하고 있네요. 현재는 코드가 복잡하지 않아 괜찮지만 코드가 복잡해지면 디버깅이 힘들 것 같습니다.
이를 방지하기 위해 final 키워드를 사용해 보는 것도 좋을 것 같습니다. :)
if (number < 0) { | ||
throw new RuntimeException("Negative numbers are not allowed: " + number); | ||
} |
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.
0 또는 양수를 뜻하는 클래스를 작성해 보는 것은 어떨까요?
private int sumTokens(String[] tokens) { | ||
int sum = 0; | ||
for (String token : tokens) { | ||
if (!token.isEmpty()) { | ||
sum += NumberParser.parse(token); | ||
} | ||
} | ||
return 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.
규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만 한다.
를 지켜보는 것은 어떨까요?
감사합니다