[자동차 경주 - 단위테스트] 2단계 - 문자열 덧셈 계산기 #5914
Merged
Conversation
This file contains 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
wheejuni
requested changes
Mar 12, 2025
| @@ -6,35 +6,4 @@ | |||
| * 모든 피드백을 완료하면 다음 단계를 도전하고 앞의 과정을 반복한다. | |||
|
|
|||
| ## 온라인 코드 리뷰 과정 | |||
| * [텍스트와 이미지로 살펴보는 온라인 코드 리뷰 과정](https://github.com/next-step/nextstep-docs/tree/master/codereview) | |||
There was a problem hiding this comment.
제가 임의로 작성한 내용이라서 필요하면 나중에 필요하면 리버트하겠습니다 ㅎ
| return sumPositiveNumbersOrThrow(tokens); | ||
| } | ||
|
|
||
| private static String[] splitIntoTokens(String text) { |
There was a problem hiding this comment.
리턴 타입을 List<String> 으로 바꿔보시는 건 어떨지요?
여러가지 의견이 있을 순 있지만 저는 primitive type을 제외한 타입들을 갖는 원소들을 담을 땐 java.util.Collection 의 구현체들을 사용하는것이 더 좋다고 생각합니다. 국인님의 생각도 궁금하네요.
There was a problem hiding this comment.
좋은 의견입니다!
처음에는 파싱하는 메소드의 반환값을 그대로 반환하려는 의도로 작성한것 같네요. 컬렉션에 실용적인 api들이 많고 다른 메소드들과 타입 호환성이 좋다고 생각하고 보통은 컬렉션으로 반환하는 편입니다.
해당 부분을 List.of()를 사용해 불변 리스트를 반환하도록 수정하였습니다.
다만, 실무에서는 다른 작업자들이 이 리스트가 불변이라는 사실을 모르고 리스트를 변경하여
런타임에서 UnsupportedOperationException을 발견하는 경우를 종종 보았습니다.
이런 실수를 줄이기 위해 몇 가지 방안을 고민 중인데요.
-
불변임을 명시적으로 표현하는 방법
List<String>대신ImmutableList<String>(Guava) 사용Collections.unmodifiableList()로 감싸기- Javadoc이나 코드 주석으로 설명 추가
-
반드시 불변 리스트를 유지해야 하는지 여부
- 현재 컨텍스트에서 불변 리스트가 필요한지, 아니면
new ArrayList<>(List.of(...))로 가변 리스트를 반환하는 것이 더 나을지?
- 현재 컨텍스트에서 불변 리스트가 필요한지, 아니면
-
팀 내 컨벤션 정리
- 불변 리스트 반환 시 명확한 패턴을 정하는 것이 좋을까요? (예: 불변 리스트는 반드시
ImmutableList사용)
- 불변 리스트 반환 시 명확한 패턴을 정하는 것이 좋을까요? (예: 불변 리스트는 반드시
혹시 이런 상황에서 어떻게 대응하는 것이 좋을지, 팀에서 따르는 규칙이 있는지 궁금합니다.
리뷰어님의 의견 부탁드립니다. 🙏
|
|
||
| public class StringAddCalculatorTest { | ||
|
|
||
| @DisplayName("빈 문자열 또는 null 값을 입력할 경우 0을 반환해야 한다.(예 : “” => 0, null => 0)") |
wheejuni
approved these changes
Mar 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
2단계 PR 리뷰 요청드립니다.