-
Notifications
You must be signed in to change notification settings - Fork 59
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주차 미션 제출합니다. #46
base: dldb-chamchi
Are you sure you want to change the base?
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.
1주차 미션 고생하셨습니다😃
테스트를 고민한 흔적이 보이는 것 같아요.
궁금증에 대한 답변은 리뷰로 남겨두었습니다.
남은 주차들도 파이팅입니다~!
src/main/java/StringCalculator.java
Outdated
} | ||
|
||
//문자열 덧셈 실행 | ||
public int sum(String input){ |
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.
저는 int를 사용할때는 항상 오버플로우를 의식해야 한다고 생각합니다.
너무 큰 숫자의 덧셈에 대해서 예외처리를 해주거나 Java에서 제공하는 BigInteger
를 사용해보시면 좋을 것 같아요!
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.
항상 오버플로우를 생각하지 못하고 코드를 짜는 경우가 많은데 짚어주셔서 감사합니다. 생각해본 결과, 보통 계산기에는 표현할 수 있는 수의 한계가 있기 때문에 그런 점을 고려하여 예외처리 하는 것으로 진행했습니다!
@Test | ||
void custom_delimiter_Test(){ | ||
StringCalculator calc = new StringCalculator(); | ||
assertEquals(6, calc.sum("//;\n1;2;3")); |
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.
아리님이 말씀해주신 궁금증이 이부분 인 것 같습니다. 혼동하고 계신 부분이 있는 것 같아요!
콘솔로 입력을 받는다면 "//;\n1;2;3"
는 아래와 같은 형태일 것입니다. 이 점을 한번 생각해보시면 좋을 것 같아요!
//;
1;2;3
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.
일자로 입력받아서 어떤 의미인지 제대로 이해하지 못했던 것 같습니다! 알려주셔서 감사합니다. 궁금증이 해결됐습니다 ㅎㅎ
src/main/java/StringCalculator.java
Outdated
} | ||
|
||
//커스텀 구분자가 있는지 확인 후 구분자대로 문자열 나누기 | ||
public String[] checkDelimiter(String input) { |
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.
커스텀 구분자가 있는지 확인 후 구분자대로 문자열 나누기
에서 구분자대로 문자열 나누기
는 checkDelimiter의 역할에서 조금 떨어져 있는 것 같습니다! 역할에 따라서 함수를 분리해 보는 것은 어떨까요?
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.
splitDelimiter 메소드를 통해 역할을 분리해보았습니다. 위의 함수에서는 구분자를 나누기 이한 인덱스를 전달하는 것으로 수정하였습니다!
src/main/java/StringCalculator.java
Outdated
|
||
//커스텀 구분자가 있는지 확인 후 구분자대로 문자열 나누기 | ||
public String[] checkDelimiter(String input) { | ||
String checkedInput = checkInput(input); |
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검사까지 꼼꼼하게 해주셨군요👍👍
null의 경우는 최대한 상위 레벨에서 미리 검사해줘서 하위 레벨에 전파되지 않도록 하는 것이 중요하다고 생각하는데요.
그런 관점에서 sum함수에서 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.
calculator 메소드를 통해 null 검사를 진행한 후 넘겨주는 형식으로 변경하였습니다!
src/main/java/StringCalculator.java
Outdated
//커스텀 구분자가 있는지 확인 후 구분자대로 문자열 나누기 | ||
public String[] checkDelimiter(String input) { | ||
String checkedInput = checkInput(input); | ||
if(checkedInput.isEmpty()) return new String[0]; //문자열이 빈 경우 |
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검사까지 꼼꼼하게 해주셨군요👍👍
null의 경우는 최대한 상위 레벨에서 미리 검사해줘서 하위 레벨에 전파되지 않도록 하는 것이 중요하다고 생각하는데요.
그런 관점에서 sum함수에서 null을 미리 검새해주면 어떨까요?
이 경우도 동일하게 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.
위와 같이 동일하게 calculator 메소드를 통해 진행하였습니다!
src/main/java/StringCalculator.java
Outdated
try{ | ||
int num = Integer.parseInt(n); | ||
if(num < 0) throw new RuntimeException("음수는 입력이 불가합니다."); | ||
sum += num; | ||
} | ||
catch(NumberFormatException msg){ | ||
throw new RuntimeException("잘못된 숫자 형식입니다."); | ||
} |
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.
문자열을 검사하고 정수로 변환하는 함수를 만들어서 사용하여 try-catch문을 모듈화 하면 for문의 깊이가 깊어지는 것을 방지할 수 있을 것 같습니다! (좀더 이쁜 코드가 될것 같아요)
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.
checkException 메소드를 통해 모듈화하였습니다!
피드백 주셔서 감사합니다!!! 그리고 피드백 해주신 내용들을 코드에 반영하면서 궁금증이 생겨 질문 드립니다. checkDelimiter 메소드를 checkDelimiter & splitDelimiter 메소드 2개로 분리하였고, 구분자가 처음 시작하는 인덱스를 splitDelimiter에 전달하였습니다. 이때 빈문자열이거나 커스텀 구분자가 없을 경우 적절한 인덱스를 던져 splitDelimiter 메소드에서 검사를 해야합니다. 또, 모든 메소드들을 총괄하는 형태의 calculator 메소드를 만들어보았는데 이런 형태가 좋은 것인지 아니면 더 괜찮은 방법이 있는지 궁금합니다. |
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 checkInput(String input){ | ||
if(input == null) throw new RuntimeException("null은 허용되지 않습니다."); | ||
return input; | ||
} |
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.
checkInput메소드에서 결과를 반환하지 않아도 될 것 같습니다!
public String checkInput(String input){ | |
if(input == null) throw new RuntimeException("null은 허용되지 않습니다."); | |
return input; | |
} | |
public void validateInput(String input){ | |
if(input == null) { | |
throw new RuntimeException("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.
그렇네요... 확실히 input을 반환하지 않고 잘못된 입력이면 예외를 발생하면 될 것 같습니다!
public int checkDelimiter(String checkedInput) { | ||
if(checkedInput.isEmpty()) return -2; //문자열이 빈 경우 | ||
|
||
|
||
if (checkedInput.startsWith("//")) { //커스텀 구분자를 설정하는가? | ||
int idx = checkedInput.indexOf("\n"); //제대로된 형식인가? | ||
if (idx == -1) throw new RuntimeException("잘못된 커스텀 구분자 형식입니다."); | ||
return idx; //구분자 위치 전달 | ||
} | ||
|
||
return -1; //기본 구분자대로 문자열을 나눔 | ||
} |
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.
커스텀 구분자가 있는지 확인하기 보단, 커스텀 구분자가 를 추출해서 반환해주면 좋을 것 같아요!
커스텀 구분자가 있으면 커스텀 구분자를 추출하고 아니면 빈 문자열 반환
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.
extractDelimiter과 같은 함수를 만들어서 입력 문자열에서 구분자를 추출하는 함수를 만들어도 좋을 것 같아요.
public int checkDelimiter(String checkedInput) { | |
if(checkedInput.isEmpty()) return -2; //문자열이 빈 경우 | |
if (checkedInput.startsWith("//")) { //커스텀 구분자를 설정하는가? | |
int idx = checkedInput.indexOf("\n"); //제대로된 형식인가? | |
if (idx == -1) throw new RuntimeException("잘못된 커스텀 구분자 형식입니다."); | |
return idx; //구분자 위치 전달 | |
} | |
return -1; //기본 구분자대로 문자열을 나눔 | |
} | |
public String extractDelimiter(String input) { | |
String delimiter = ",:"; | |
if (checkedInput.startsWith("//")) { //커스텀 구분자를 설정하는가? | |
int index = checkedInput.indexOf("\n"); //제대로된 형식인가? | |
if (index == -1) throw new RuntimeException("잘못된 커스텀 구분자 형식입니다."); | |
delimiter += input.subString(커스텀구분자 시작index, 커스텀구분자 종료index); | |
} | |
return 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.
checkDelimiter에 맞는 내용만 생각해서 구분자가 있는지 확인하는 함수로만 만들려고 생각한것 같습니다. 알려주셔서 감사합니다!
} | ||
|
||
//문자열 덧셈 실행 | ||
public int sum(String[] strings){ |
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.
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.
추가로 인자로 주어진 수를 더하는 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.
확실히 sum에서는 덧셈만 하는게 맞을거 같습니다! 피드백 감사합니다!
1주차 미션 제출합니다.
<실수>
git commit 목록에 dldb-chamchi가 아닌 chamchicom이라는 닉네임이 있습니다. 이건 제가 git 처음 설정 떄 잘못 로그인한걸 모르고
commit 해버려서 그런 것 입니다. 이미 push후라 혹시 모를 오류가 있을 수 있어 수정하지 않았습니다. 다음부터는 조심하겠습니다.
이번 문자열 계산기를 진행하면서 문자열 구분법, 정규표현식, 테스트 작성 법 등을 알 수 있었습니다. 특히 정규표현식이나 문자열을 다루는게 아직 미숙해서 많이 어려웠습니다. 그리고 관련해서 질문이 있습니다.
<궁금점>
제가 처음 이 미션을 만났을 떄 \n의 인덱스를 찾기 위해 index.of("\ \n")형식으로 찾았는데 이렇게 하니 인덱스를 제대로 찾지 못했습니다. 그래서 indexOf("\n")로 작성하였고 다른 글을 참고해보니 indexOf("\n") 이 형태가 맞다고 하는데 이게 정말 맞는 것인지 아니면 제가 놓친 것이 있는지 궁금합니다.
<느낀점>
이번 미션에는 Junit5와 AssertJ를 통해 총 두개의 테스트를 작성 했습니다. 제가 처음 자바를 배울 때 사용한 테스트 프로그램은 Junit이라서 이번 과제를 작성할 때 Junit이 더 편했습니다. 하지만 AssertJ도 테스트를 작성하면서 어느정도 익숙해지다보니 가독성도 좋고 편리하다고 생각했습니다. 앞으로는 Junit과 AssertJ를 번갈아가면서 테스트를 작성해보려고 합니다.
또 처음으로 main함수를 쓰지 않고 코드를 작성하는 것이 굉장히 어색했습니다. 하지만 main없이 코드를 작성하고 테스트 프로그램을 통해 코드의 오류를 보거나 제대로 작동하는지 검사했을 때는 굉장히 편했습니다. main을 통해 작성하고 출력을 통해 확인 했을 때보다 확실히 편하다는 것을 실감했습니다.