[문자열 계산기] 표현록 미션 제출합니다.#1
Open
Regyung wants to merge 5 commits into
Open
Conversation
앞의 기본 구분자(쉼표, 콜론) 외에 커스텀 구분자를 지정할 있다.커스텀 구분자는 문자열 앞부분의 "//"와 "\n" 사이에 위치하는 문자를 커스텀 구분자로 사용한다.
사용자가 잘못된 값을 입력할 경우 `IllegalArgumentException`을 발생시킨 후 애플리케이션은 종료되어야 한다.
moongua404
reviewed
Feb 9, 2025
|
|
||
| System.out.printf("결과 : %d%n", sum); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
이 파일도 주석을 달지 말고 그 부분마다 함수를 분리하는게 더 좋을 것 같아요
| // 구분자 정리 | ||
| String[] operatorArray = new String[operatorList.size()]; | ||
| for (int i = 0; i < operatorList.size(); i++) | ||
| operatorArray[i] = operatorList.get(i); |
Contributor
There was a problem hiding this comment.
인덱스로 접근하기보다는 원소로 접근하는게 좋을 것 같아요! 1별다른 이유가 없다면 operatorArray도 배열이 아니라 컬렉션, 가령 ArrayList 등으로 표현하면 확장하기 쉬울 것 같아요
| if (i <= 0) | ||
| throw new IllegalArgumentException(); | ||
| sum += i; | ||
| } |
Contributor
There was a problem hiding this comment.
이 부분을 stream으로 표현하면 가독성이 더 높고 효율적일 것 같아요
| if (m.find()) { | ||
| operatorList.add(String.valueOf(word.charAt(m.start()+2))); | ||
| word = word.substring(m.end()); | ||
| } |
Contributor
There was a problem hiding this comment.
정규표현식으로 처리한 김에 좀 더 확장해서 정규표현식으로 추출하도 좋을 것 같아요
| // 기본 구분자 설정 | ||
| ArrayList<String> operatorList = new ArrayList<>(); | ||
| operatorList.add(","); | ||
| operatorList.add(":"); |
Contributor
There was a problem hiding this comment.
이부분도 List.of로 한 번에 추가할 수 있을 것 같아요!
| operatorArray[i] = operatorList.get(i); | ||
| String operator = String.join("|", operatorArray); | ||
|
|
||
| // 숫자 검수 및 연산 |
There was a problem hiding this comment.
저도 처음이라 미숙해서 함수별로 분리 안하였는데 분리하는게 좋을 것 같습니다
| Pattern p = Pattern.compile("//\\D\\\\n"); | ||
| Matcher m = p.matcher(word); | ||
| if (m.find()) { | ||
| operatorList.add(String.valueOf(word.charAt(m.start()+2))); |
| // 구분자 정리 | ||
| String[] operatorArray = new String[operatorList.size()]; | ||
| for (int i = 0; i < operatorList.size(); i++) | ||
| operatorArray[i] = operatorList.get(i); |
There was a problem hiding this comment.
저도 배열로 하다가 중간에 ArrayList로 변경했는데 ArrayList가 더 좋아보여요!
| String[] result = word.split(operator); | ||
| for (String s : result) { | ||
| int i = Integer.parseInt(s); | ||
| if (i <= 0) |
There was a problem hiding this comment.
저는 입력조건에 양수라고 되어있어 따로 0과 음수에 대한 예외처리를 하지않았는데 수정할 때 참고해볼게요..! 예외처리를 신경쓰신 것 같아요!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
신경써서 구현한 부분
피드백이 필요한 부분
혼자 코딩할 당시에는 몰랐는데 첫 리뷰를 받고 나니 많이 부족하다는 걸 느꼈습니다.
고려한 예외
너무 생각을 얕게 했던 것 같습니다...