최건위 sprint3#62
Hidden character warning
Conversation
|
|
hagyutae
left a comment
hagyutae
left a comment
There was a problem hiding this comment.
미구현 내용이 있는건 괜찮은데, 컴파일은 가능한 상태로 올려주셔야 리뷰가 가능해요.
우선, 컴파일 오류 나는 부분에 대해서는 IDE에서 바로 확인할 수 있으니 특별히 더 코멘트 하지 않았습니다.
작업 마무리해주세요~!
| public class BasicUserService implements UserService { | ||
| private final UserRepository userRepository; | ||
| private final UserStatusRepository userStatusRepository; | ||
| private final BinaryContentRepository binaryContentRepository; |
There was a problem hiding this comment.
해당 인터페이스가 diff에 존재하지 않습니다. 컴파일 오류가 발생합니다.
| private Instant lastReadAt; | ||
|
|
||
| public ReadStatus(UUID id, UUID userId, UUID channelId) { | ||
| this.id = UUID.randomUUID(); |
There was a problem hiding this comment.
전달받은 UUID를 사용하지 않으면, 생성자 인자로 전달받는 의미가 없어요.
그리고 맥락상, ReadStatus ID는 생성자 내부에서 새로 생성하는게 맞는데, 나머지는 전달 받은 값들을 할당해야 합니다.
| if (id == null) throw new IllegalArgumentException("id must not be null"); | ||
| if (bytes == null) throw new IllegalArgumentException("bytes must not be null"); | ||
|
|
||
| this.id = UUID.randomUUID(); |
There was a problem hiding this comment.
검증 후 덮어쓰기라는 로직이 일단 논리적이지 않구요,
여기서도 id 값은 외부에서 전달받기보다 직접 생성해서 할당하는게 맞습니다!
| @@ -0,0 +1,55 @@ | |||
| //package com.sprint.mission.discodeit; | |||
| ChannelRepository channelRepository = context.getBean(ChannelRepository.class); | ||
| MessageRepository messageRepository = context.getBean(MessageRepository.class); | ||
|
|
||
| User user = setupUser(userRepository); |
There was a problem hiding this comment.
Repository는 App에서 직접 사용하시면 안돼요.
우리는 앞으로 Service 통해서 작업을 수행할거고, 그 과정에서 필요하다면 Service가 Repository를 활용하는 형태가 되어야 합니다!
|
|
||
| System.out.println("채널 전체조회: " + channelRepository.findAll().size()); | ||
|
|
||
| Channel updated = new Channel(null,"공지사항","공지사항입니다."); |
There was a problem hiding this comment.
이건 updated가 아니라 create를 하고있는건데, 엔티티는 Service한테 생성을 위임해야지, 이렇게 직접적으로 객체화 시키면 안돼요.
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| @Configuration | ||
| public class ServiceConfig { |
There was a problem hiding this comment.
요구사항은 @Service, @Repository를 각 계층에 맞게 사용하라고 되어 있으나, @Configuration + @Bean 방식을 사용하고 있습니다. BasicMessageService만 @Service가 붙어있고 나머지는 @Configuration에서 @Bean으로 등록합니다. 일관성이 없습니다.
- 의견: 두 방식 모두 유효하지만, 요구사항의 의도는
@Service,@Repository어노테이션을 사용하는 것입니다.@Configuration+@Bean은 심화 요구사항의@ConditionalOnProperty와 함께 사용하는 것이 더 적합합니다.
2 participants
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게