[1단계 - 사다리 생성] 에단(김석호) 미션 제출합니다.#107
Conversation
iphwade
left a comment
iphwade
left a comment
There was a problem hiding this comment.
안녕하세요-!
코드 리뷰를 하게된 코일입니다.
전체적으로 짜임새있고 클래스 분리, 메서드 분리가 잘 되어있어서 리뷰드릴 부분이 적네요-!
고생하셨습니다-!
사용자의 입력 처리 로직은 Controller, InputView 중에서 어떤 곳에 위치를 시켜야 할까요?
-> 예외 처리에 적절한 클래스가 어디일지를 고민해보면 좋을 것 같습니다. Controller에 모든 예외가 모이면 좋은 이유가 뭐라고 생각하실까요-?! 반대로 발생하는 곳에서 가장 가까운 InputView에서 처리하면 좋은 점이 뭐라고 생각하실까요-?!
원시값을 포장할 때 어느 정도까지 포장을 해야하는지 궁금합니다.
-> 1을 상수화하는 것이 원시값을 포장을 의미하는 것은 아니라고 생각합니다. 원시값의 포장은 해당 원시값이 충분한 메서드 (역할)을 가질 때 필요합니다. 말씀하신 예시는 매직넘버를 어떻게 처리할지에 대한 질문인 것 같습니다-! lastIndex로 네이밍된 변수가 충분히 표현이 된다고 생각되어 지금은 괜찮다고 보여집니다-!
다른 질문이 있으시면 편하게 DM주세요-!
| static class NameLengthTest { | ||
| @DisplayName("1 이상 5 이하 글자가 들어오면 name이 정상적으로 생성된다.") | ||
| @ParameterizedTest | ||
| @ValueSource(strings = {"에단", "준팍"}) |
|
|
||
| //then | ||
| Assertions.assertThat(result) | ||
| .extracting("names") |
There was a problem hiding this comment.
view에서 getter를 사용하기 때문에 테스트에서도 getter를 사용하도록 수정했습니다.
| @DisplayName("너비가 1 미만이면 예외가 발생한다") | ||
| void givenOneLessWidth_thenFail() { | ||
| assertThatThrownBy(() -> new Line(0)) | ||
| assertThatThrownBy(() -> Line.from(0)) |
| try { | ||
| random = SecureRandom.getInstanceStrong(); | ||
| } catch (NoSuchAlgorithmException e) { | ||
| System.out.println(e.getMessage()); |
There was a problem hiding this comment.
처음에는 try-catch하고 에러를 프린트하는 것이 익숙해서 이렇게 만들었었습니다.😅 하지만 생각해보니 프린트 문을 없애는게 더 적절하다고 생각이 듭니다. 에러를 프린트해서 ‘사용자’에게 알려주는 것은 사용자의 입력이 잘못되었을 때 사용자에게 인식을 시키기 위해서입니다. 현재 작성된 코드를 보면 예외가 발생하는 경우는 랜덤 객체가 생성되지 않는 경우 입니다. 그렇다면 이 경우는 사용자의 입력과 관련 없는 서버 문제가 됩니다. 따라서 이 코드에서 사용하는 print는 사용자에게 알려주는 것이 아니라 log의 의미로 해석이 될 수 있기 때문에 없어지는 것이 적절하다고 생각이 됩니다.
| public List<Line> getLines() { | ||
| return Collections.unmodifiableList(lines); | ||
| } |
There was a problem hiding this comment.
Getter 컨벤션이 무엇인지 알 수 있을까요?ㅜㅜ 찾아봐도 정보가 나오지 않네요. 관련해서 학습할 자료가 있다면 추천해주세요!
There was a problem hiding this comment.
https://dodop-blog.tistory.com/277를 기준으로 삼으면 좋아보입니다-!
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = {"에단", "준팍", "또링", "코일", "블랙캣", "백여우"}) |
|
|
||
| private void validateNUmberOfNames(final List<String> names) { | ||
| if (names.size() < 2) { | ||
| throw new IllegalArgumentException(); |
There was a problem hiding this comment.
일단은 돌아가는 코드를 만들고 수정하자라는 생각을 가지고 코딩을 했습니다!! 지금은 에러메세지가 있는 상태로 수정되었습니다😄
| public List<Boolean> getStatuses() { | ||
| return new ArrayList<>(statuses); | ||
| } |
There was a problem hiding this comment.
방어적 복사를 함으로써 원본 객체를 보호하기 위해서 입니다. 물론 방어적 복사가 깊은 복사가 아니기 때문에 완벽히 방어할 수는 없습니다. 다만 getter를 쓰는 곳에서 추가나 삭제 시 원본을 방어하는 것도 충분히 의미가 있어보여서 사용했습니다.
|
안녕하세요. 코일! 반영사항에 대해서 코멘트 남겼습니다. 그리고 질문에 대해서 제 생각을 남깁니다.
|
3 participants
안녕하세요😄 코일! 이번에 준팍(박준현)과 페어로 같이한 사다리 미션 제출합니다!
이번 미션에서는 SRP 원칙과 모든 원시값을 포장하려고 노력했습니다. 코드 리뷰 미리 감사드립니다.
추가로 궁금한 프로그래밍을 하면서 궁금한 사항이 있어서 질문을 남깁니다!