Conversation
| public static ChannelResponse entityToDto(Channel channel, Instant lastMessageTime, List<UUID> joinUsers) { | ||
| return new ChannelResponse(channel.getId(), channel.getTitle(), channel.getDescription(), channel.getChannelType(), channel.getCreatedAt(), channel.getUpdatedAt(), lastMessageTime, joinUsers); | ||
| } | ||
| } |
There was a problem hiding this comment.
dto 안 메소드로 정의해서 변환하고 있습니다.
변환 메소드를 dto 안에서 정의하면 오히려 사용하기 쉬울 것 같았는데, 찾아보니 mapper 클래스를 별도로 만들어 관리하는 것 같습니다. 관련기능을 관리하기에는 mapper 클래스로 분리하는 게 나을까요?
There was a problem hiding this comment.
이건 정답이 있는 문제는 아닙니다.
modelMapper나 mapStruct를 사용하는 것이 생산성이 좋다!
라고 하는 분들이 계실거고,
modelMapper나 mapStruct를 사용해서 변환하는 것은 생산성에 큰 영향이 없고 관리해야 하는 대상이 더 늘어난다!
라고 하시는 분들이 계십니다.
멘토의 입장에서, 그리고 실무에서 일을 하고 있는 제 입장으로는 '한번쯤은 배우고 갈 만 하지만, 굳이 사용할 필요는 없다'인 것 같아요
몇가지 이유가 있는데요,
- DTO는 본래 변환을 위한 객체
- 엔티티와 DTO 변환 생산성이 크지 않다.
- mapper를 빈으로 등록하여 스프링 관리 대상에 놓아야 한다.
사실 마지막 이유가 가장 꺼려지는 것 같아요. 빈으로 등록하게 되면 결국에는 의존관계를 신경써야 하는 것인데 운영 환경에서 이런 부분까지 신경을 써야 한다는게 조금 부담이 되는 것 같아요
지금은 단순한 스프린트 기간이니 한번쯤은 써보시고, 각각의 장단점과 성능을 비교해보시는 것도 좋을 것 같아요 :D
(개인적으로 아직 실무에서 Mapper를 쓰는 팀이나 회사는 못본것 같아요. 제가 2년차라서 그럴수도 있고요)
| public record MessageRequest() { | ||
| public record Create( | ||
| String content, | ||
| UUID channelId, | ||
| UUID userId, | ||
| List<MultipartFile> files | ||
| ) {} | ||
|
|
||
| public record Update( | ||
| String content, | ||
| List<MultipartFile> files | ||
| ) {} | ||
| } |
There was a problem hiding this comment.
사실 dto 코드는 길지 않고 서로 관련이 있다고 생각해서 하나의 클래스 안에 넣었습니다. 실제로도 이렇게 많이 사용하시는지 궁금합니다.
생각해보니 계속 기능이 늘어나면 dto를 사용하는데 있어서 오히려 복잡해질 수도 있을 것 같습니다
There was a problem hiding this comment.
네, 참고로 저도 저렇게 쓰고 있습니다 :D record를 쓰진 않고, 인터페이스에 inner class로 쓰고는 있긴 하지만, 저런 방식도 괜찮은 것 같아요.
어차피 대부분 CRUD라서, 도메인 통채로 Response, Request를 하나의 파일로 관리하는 것 아니면 그렇게 크게 늘어나지도 않을 수 있어요
이번 스프린트가 아마 DTO에 대한 내용을 배우셨던 것 같은데요, record를 쓰시기 전에 히스토리를 배우고 가시면 좋을 것 같아요. (시온님이랑 태식님한테도 같은 리뷰를 드렸어요.)
예전에는 record를 Request DTO로 사용할 때는 상당히 불편한(?) 문제를 마주칠 수 있습니다.
- Jackson 역직렬화 문제 (Deserialization Issue)
Spring에서는 JSON 요청을 객체로 변환할 때 Jackson을 사용해 역직렬화를 수행하는데요. record에서는 기본적으로 기본 생성자가 따로 없습니다. 그래서 아래와 같이 작성해주셔야 합니다.
public record UserRequest(
@JsonProperty("username") String username,
@JsonProperty("password") String password,
@JsonProperty("email") String email,
@JsonProperty("phoneNumber") String phoneNumber,
@JsonProperty("profileImageId") UUID profileImageId
// @JsonProperty("profileImageName") String profileImageName
) {
// @JsonCreator를 추가하여 명시적으로 생성자를 지정
@JsonCreator
public UserRequest(
@JsonProperty("username") String username,
@JsonProperty("password") String password,
@JsonProperty("email") String email,
@JsonProperty("phoneNumber") String phoneNumber,
@JsonProperty("profileImageId") UUID profileImageId
// @JsonProperty("profileImageName") String profileImageName
) {
this.username = username;
this.password = password;
this.email = email;
this.phoneNumber = phoneNumber;
this.profileImageId = profileImageId;
// this.profileImageName = profileImageName; // 주석처리된 부분도 필요에 맞게 추가 가능
}
}뭔가 상당히 불편해 보이시지 않나요 ^-^..?
@JsonCreator를 통해서 생성자를 명시적으로 지정하여 Jackson이 record 객체를 생성할 때 사용할 생성자를 알 수 있도록 해야 하고, @JsonProperty를 사용해서 JSON 필드 이름과 record 필드명을 매핑시켜줘야 역직렬화가 이루어지게 됩니다.
이러한 이유들 때문에 예전에는 안쓰는 팀도 많았습니다. 단순히 class에 Lombok 어노테이션을 쓰면 간소화 할 수 있기 때문이지요.
그런데 JDK 14부터는 그런 문제들이 해결되었다고 하더라구요. 그렇기 때문에 Record 도입은 버전을 잘 고려하시면서 적용시키면 좋을 것 같습니다
(나중에 프로젝트를 하시고, 면접에 가셨을 때 DTO를 record를 쓰신 이유를 물어보거나 역직렬화 이슈에 대해 면접관들이 물어보면, 잘 대답해주시면 면접관들이 좋아 죽습니다 😄)
| private ParentType parentType; | ||
| private UUID userId; | ||
| private UUID messageId; |
There was a problem hiding this comment.
필드를 정의할 때 고민했었던 부분이 있습니다.
처음에는 parentType과 parentId로 구성하려고 했었습니다.
'UUID로 id 값이 같으니 오히려 parentId로만 구성한다면 값을 저장할 때 편하지 않을까?' 라고 생각했었습니다.
그래도 걸리는 부분이 2가지가 있었는데요.
- 데이터를 가져올 때 항상 parentType을 확인해야하고 잘못된 파일에서 값을 가져올 수도 있겠다는 부분이었습니다.
- 나중에 데이터베이스로 연결할 때, 참조되려면 결국 나누는게 맞다고 생각되더라구요. 그런데 생각했던 건 userId와 messageId가 구분된다면 parentType 필드가 굳이 필요할까 라는 생각이 들기도 합니다..
There was a problem hiding this comment.
저라면 parentId값만 가지고 올 것 같아요 :D 필요할때마다 즉각즉각 조회하면 되니깐요
(1~2번 추가 쿼리는 성능적으로 큰 영향이 없습니다, 오히려 다른 부분에 더 많은 영향을 끼치기 때문에)
만약에, parentId의 type이 변경이 된다면 당연히 BinaryContent도 조회를 하고 변경을 해줘야 하기 때문에 더 신경써야 하는 부분이 많지 않을까요
There was a problem hiding this comment.
아 그러네요 ㅎㅎ,,
사실 파일만 가지고 오면 되는데 Message파일인지 , User파일 구분할 필요가 없을 것 같습니다 😂
자꾸 분류해야 한다는 생각을 가지고 있었던 것 같습니다.
| public static BinaryContent createBinaryContent(String fileName, String contentType, byte[] bytes, ParentType parentType, UUID parentId) { | ||
| if (parentType == ParentType.USER) { | ||
| return new BinaryContent(fileName, contentType, bytes, parentType, parentId, null); | ||
| } else { | ||
| return new BinaryContent(fileName, contentType, bytes, parentType, null, parentId); | ||
| } | ||
| } |
There was a problem hiding this comment.
일단 parentType을 사용하기로 하고 생성자를 만들었습니다. 생성할 때, parentType을 조건으로 두고 생성하고 있습니다.
그런데 entity 안에서 이 로직을 쓰는게 괜찮은 건지 모르겠습니다.
지금 보니 엔티티의 책임이 너무 많은 것 같아 아래 두 가지 방법을 생각했는데요.
- 엔티티 안에서
createUserBinaryContent,createMessageBinaryContent로 분리하기 - 엔티티는 단순히 생성하고 service 에서 처리하기
다시 정리해서 쓰다보니 둘 다 적용하면 되겠다는 생각이 드네요 ㅎㅎ..
정적 팩토리 메서드를 쓰는 이유를 생각해보면 첫 번째를 적용해야할 것 같고, 타입 조건은 service에서 처리해야 할 것 같습니다.
There was a problem hiding this comment.
둘 다 틀린 방식은 아닙니다!
생성만 엔티티에 할 것인지, 타입을 보고 생성하는 역할까지 엔티티에게 줄 것인지는 정말 스타일 차이인 것 같아요.
(보통 저는 생성 자체는 엔티티에, 그리고 타입을 보고 어떤 객체를 생성해야하는지는 서비스 계층에서 하는 것 같아요)
Enum을 조금 더 확장해서 다형성을 조금 이용해보면 이런 방식도 있을 것 같아요.
public enum ParentType {
USER {
@Override
public BinaryContent createBinaryContent(String fileName, String contentType, byte[] bytes, UUID parentId) {
return new BinaryContent(fileName, contentType, bytes, this, parentId, null);
}
},
MESSAGE {
@Override
public BinaryContent createBinaryContent(String fileName, String contentType, byte[] bytes, UUID parentId) {
return new BinaryContent(fileName, contentType, bytes, this, null, parentId);
}
};
public abstract BinaryContent createBinaryContent(String fileName, String contentType, byte[] bytes, UUID parentId);
}// 호출부
BinaryContent binaryContentForUser = ParentType.USER.createBinaryContent(xxx);어떤 방식이든 좋습니다. 예림님이 생각이 담겨있는 코드로 작성해주세요 :D
| public void update(String newName, String newEmail, String newPassword) { | ||
| boolean isChanged = false; | ||
| if (newName != null && !newName.equals(this.name)) { | ||
| this.name = newName; | ||
| isChanged = true; | ||
| } | ||
| if (newEmail != null && !newEmail.equals(this.email)) { | ||
| this.email = newEmail; | ||
| isChanged = true; | ||
| } | ||
| if (newPassword != null && !newPassword.equals(this.password)) { | ||
| this.password = newPassword; | ||
| isChanged = true; | ||
| } | ||
|
|
||
| public void removeChannel(Channel removeChannel) { | ||
| channelList.remove(removeChannel); | ||
| if (isChanged) { | ||
| this.updatedAt = Instant.now(); | ||
| } |
There was a problem hiding this comment.
기존에 모든 업데이트 분리되어 있어서 updateAt이 여러 번 갱신 되는 부분을 코드를 참고하여 변경하였습니다. 모든 필드마다 update문이 나누어진 것 보다 메서드 하나의 모아 처리하는 방식이 좋은 것인지?는 잘 모르겠습니다... 멘토님은 어떤식으로 하시나요??
There was a problem hiding this comment.
null 체크를 엔티티 즉 도메인 내에서 해야하는 이유가 따로 있을까요?
만약에 객체의 특정 타입으로 인해서 반드시 null로 세팅을 해야하는 거라면 예림님 방식을 채택할 것 같은데,
단순히 파라미터들이 null인지에 대해서 체크하는 거라면 저는 Validator에서 처리하거나 서비스 로직에서 처리할 것 같아요.
public void update(String newName, String newEmail, String newPassword) {
this.name = newName;
this.email = newEmail;
this.password = newPassword;
this.updatedAt= LocalDateTime.now();
}There was a problem hiding this comment.
service에서 받을 때는 validation으로 null 체크를 하고 있습니다.
entity에서의 null은 기존값과 다르지 않으면 service에서 update 매개변수에 null을 넣어주는 방식을 쓰고 있기 때문입니다 ,,ㅎㅎ
다시보니 entity안에서 기존값과 비교하고 있는데, service에서 한 번 더 비교하고 있었네요.
기존 값과 비교하고 다른 값이 있으면 해당 필드만 변경하도록 하는 의도로 했었습니다!
갱신할 필요가 없는 필드는 접근하지 않으려고 했던 것 같습니다.
| public static UserStatus createUserStatus(UUID userId) { | ||
| return new UserStatus(userId); | ||
| } | ||
|
|
||
| private UserStatus(UUID userId) { | ||
| this.id = UUID.randomUUID(); | ||
| this.createdAt = Instant.now(); | ||
| this.updatedAt = this.createdAt; | ||
| this.status = Status.ONLINE; | ||
| } |
There was a problem hiding this comment.
이 부분도 createUserStatus에서 매개변수로 값을 넣어주는 게 맞는 것 같습니다,,ㅎ
제가 정적 팩토리 메서드를 제대로 적용하지 못한 것 같습니다
| public Status getStatus() { | ||
| updateStatus(); | ||
| return this.status; | ||
| } | ||
|
|
||
| public void updateStatus() { | ||
| Instant ValidTime = this.updatedAt.plusSeconds(ADDITIONAL_TIME_SECONDS); | ||
| if (ValidTime.compareTo(Instant.now()) > 0) { | ||
| this.status = Status.ONLINE; | ||
| } else { | ||
| this.status = Status.OFFLINE; | ||
| } | ||
| } |
There was a problem hiding this comment.
상태를 얻어올 때 시간에 따라 자동으로 갱신 되어야 한다고 생각해서 entity 안에서 구현했던 것 같습니다. 시간을 직접 갱신하는게 아니라면 serviece로 빼야하는게 맞을까요?
| private static final String FILE_PATH = "binary_content.ser"; | ||
| private final FileManager<BinaryContent> fileManager = new FileManager<>(FILE_PATH); |
There was a problem hiding this comment.
의존성 부분을 고려해서 변경하려고 했는데, bean을 등록하려고 하다보니 제네릭 클래스를 어떻게 해야할지 잘 모르겠어서 변경하지 못했습니다. 공부해서 config 파일에서 설정해야 할 것 같습니다.
| BinaryContent createUserProfileFile(MultipartFile file, UUID userId); | ||
| BinaryContent createMessageFile(MultipartFile file, UUID messageId); | ||
| BinaryContent updateUserProfileFile(MultipartFile file, UUID userId); | ||
| BinaryContent findById(UUID id); | ||
| List<BinaryContent> findAllByIdIn(List<UUID> ids); | ||
| void deleteById(UUID id); | ||
| void deleteByUserId(UUID userId); | ||
| void deleteAllByMessageId(UUID messageId); |
There was a problem hiding this comment.
항상 메서드명이나 필드명이 고민이 많이 되는 것 같습니다.
지금은 repository와 비슷하게 해놓았는데요.
멘토님은 service 메서드명을 지으실 때, 어떤 식으로 정하시나요??
deleteMessage , deleteMessageById, deleteById 처럼 여러 방식이 있는데, 어떤 객체인지를 확실히 들어내는지, 아니면 이미 어떤 서비스인지 보이니 필요한 매개변수 정보를 넣으시는지 궁금합니다
There was a problem hiding this comment.
저도 사실 네이밍에 시간을 많이 투자하는 편입니다 ^-^;;
(가끔씩은 chatgpt한테 추천도 받습니다😢)
저는 좀 규칙이 있는 것 같아요
조회 : getByXXX
생성 : createByXXX
...
만약에 여러건이다 하면
getAllByXXX
등등 쓰는 것 같아요.
확실한건 예를 들어 ContentService라고 했을 때,
get**Content**ByXXX
해당 클래스의 도메인은 중복으로 하지 않는 것 같아요.
또한, 엔티티에서 id값(pk든, fk든)을 여러개 들고 있다면 파라미터로 추상화를 풀어주는 것 같아요
ex)
getById(Long Id) // 도메인이 content이니깐 contentId는 사용 X
getByparentId(Long parentId) // 같은 파라미터일 때는 오버로딩이 안되기 때문에
뭐 이런식으로 하는 것 같아요!
| private final UserStatusRepository userStatusRepository; | ||
| // private final UserService userService; 순환 참조 발생 |
There was a problem hiding this comment.
제가 서비스 레이어끼리 데이터를 주고 받으려고 할 때 순환 참조 문제가 있었습니다.
예를 들어 유저 서비스에서 메세지 정보가 필요할 때, 바로 메세지 레파지토리에서 사용해도 되는지 궁금합니다.
사실 처음에는 각각의 서비스에서 각각의 레파지토리만을 접근할 수 있어야 한다고 생각했습니다. 여러 서비스에서 하나의 레파지토리를 접근한다면 결합도 부분에서 문제가 생길 수 있다 라는 생각 때문이었습니다.
하지만 구현하다보니 문제가 생겼습니다.
UserStatus 서비스에서 유저가 존재하는지 확인하려고 할 때, 유저 서비스를 거치려고 하니 순환 참조가 발생했습니다. 유저 서비스에서 UserStatusService를 사용하고 있기 때문이었습니다.
이런 문제가 생기다 보니 굳이 service 레이어를 거쳐야 할지 고민이 생겼습니다. service 레이어끼리만 통신하는게 맞는 것 같은데,, 어떻게 해야할지 모르겠습니다.
추가적으로, service 레이어끼리 통신할 때에도 dto 로 감싸서 해야할까요?
There was a problem hiding this comment.
이것도 팀바팀, 회바회입니다. 이건 멘토링 시간에 다같이 들으면 좋을 것 같으니 그때 이야기해봐요 :D
There was a problem hiding this comment.
예림님 안녕하세요~! 🙇
이번 스프린트에는 저번에 리뷰 드린 내용들을 잘 반영해주신 모습이 인상적이였습니다 👍 코드에서도 많은 부분을 고민하고, 많은 이슈들도 마주친 게 느껴집니다.
너무 정답을 알려드리는 경향이 많은 것 같아서 꼭 다시 한 번 고민해보시는걸 추천드립니다. 고민을 하는 시간만큼 머릿속에 남기 때문이죠
이번 스프린트 너무너무 고생 많으셨고, 혹시 코멘트 관련하여 궁금하거나 이해가 안되는 점은 멘토링때 들고오시면 언제든 답변드리도록 하겠습니다 :D
| public static ChannelResponse entityToDto(Channel channel, Instant lastMessageTime, List<UUID> joinUsers) { | ||
| return new ChannelResponse(channel.getId(), channel.getTitle(), channel.getDescription(), channel.getChannelType(), channel.getCreatedAt(), channel.getUpdatedAt(), lastMessageTime, joinUsers); | ||
| } | ||
| } |
There was a problem hiding this comment.
이건 정답이 있는 문제는 아닙니다.
modelMapper나 mapStruct를 사용하는 것이 생산성이 좋다!
라고 하는 분들이 계실거고,
modelMapper나 mapStruct를 사용해서 변환하는 것은 생산성에 큰 영향이 없고 관리해야 하는 대상이 더 늘어난다!
라고 하시는 분들이 계십니다.
멘토의 입장에서, 그리고 실무에서 일을 하고 있는 제 입장으로는 '한번쯤은 배우고 갈 만 하지만, 굳이 사용할 필요는 없다'인 것 같아요
몇가지 이유가 있는데요,
- DTO는 본래 변환을 위한 객체
- 엔티티와 DTO 변환 생산성이 크지 않다.
- mapper를 빈으로 등록하여 스프링 관리 대상에 놓아야 한다.
사실 마지막 이유가 가장 꺼려지는 것 같아요. 빈으로 등록하게 되면 결국에는 의존관계를 신경써야 하는 것인데 운영 환경에서 이런 부분까지 신경을 써야 한다는게 조금 부담이 되는 것 같아요
지금은 단순한 스프린트 기간이니 한번쯤은 써보시고, 각각의 장단점과 성능을 비교해보시는 것도 좋을 것 같아요 :D
(개인적으로 아직 실무에서 Mapper를 쓰는 팀이나 회사는 못본것 같아요. 제가 2년차라서 그럴수도 있고요)
| public record MessageRequest() { | ||
| public record Create( | ||
| String content, | ||
| UUID channelId, | ||
| UUID userId, | ||
| List<MultipartFile> files | ||
| ) {} | ||
|
|
||
| public record Update( | ||
| String content, | ||
| List<MultipartFile> files | ||
| ) {} | ||
| } |
There was a problem hiding this comment.
네, 참고로 저도 저렇게 쓰고 있습니다 :D record를 쓰진 않고, 인터페이스에 inner class로 쓰고는 있긴 하지만, 저런 방식도 괜찮은 것 같아요.
어차피 대부분 CRUD라서, 도메인 통채로 Response, Request를 하나의 파일로 관리하는 것 아니면 그렇게 크게 늘어나지도 않을 수 있어요
이번 스프린트가 아마 DTO에 대한 내용을 배우셨던 것 같은데요, record를 쓰시기 전에 히스토리를 배우고 가시면 좋을 것 같아요. (시온님이랑 태식님한테도 같은 리뷰를 드렸어요.)
예전에는 record를 Request DTO로 사용할 때는 상당히 불편한(?) 문제를 마주칠 수 있습니다.
- Jackson 역직렬화 문제 (Deserialization Issue)
Spring에서는 JSON 요청을 객체로 변환할 때 Jackson을 사용해 역직렬화를 수행하는데요. record에서는 기본적으로 기본 생성자가 따로 없습니다. 그래서 아래와 같이 작성해주셔야 합니다.
public record UserRequest(
@JsonProperty("username") String username,
@JsonProperty("password") String password,
@JsonProperty("email") String email,
@JsonProperty("phoneNumber") String phoneNumber,
@JsonProperty("profileImageId") UUID profileImageId
// @JsonProperty("profileImageName") String profileImageName
) {
// @JsonCreator를 추가하여 명시적으로 생성자를 지정
@JsonCreator
public UserRequest(
@JsonProperty("username") String username,
@JsonProperty("password") String password,
@JsonProperty("email") String email,
@JsonProperty("phoneNumber") String phoneNumber,
@JsonProperty("profileImageId") UUID profileImageId
// @JsonProperty("profileImageName") String profileImageName
) {
this.username = username;
this.password = password;
this.email = email;
this.phoneNumber = phoneNumber;
this.profileImageId = profileImageId;
// this.profileImageName = profileImageName; // 주석처리된 부분도 필요에 맞게 추가 가능
}
}뭔가 상당히 불편해 보이시지 않나요 ^-^..?
@JsonCreator를 통해서 생성자를 명시적으로 지정하여 Jackson이 record 객체를 생성할 때 사용할 생성자를 알 수 있도록 해야 하고, @JsonProperty를 사용해서 JSON 필드 이름과 record 필드명을 매핑시켜줘야 역직렬화가 이루어지게 됩니다.
이러한 이유들 때문에 예전에는 안쓰는 팀도 많았습니다. 단순히 class에 Lombok 어노테이션을 쓰면 간소화 할 수 있기 때문이지요.
그런데 JDK 14부터는 그런 문제들이 해결되었다고 하더라구요. 그렇기 때문에 Record 도입은 버전을 잘 고려하시면서 적용시키면 좋을 것 같습니다
(나중에 프로젝트를 하시고, 면접에 가셨을 때 DTO를 record를 쓰신 이유를 물어보거나 역직렬화 이슈에 대해 면접관들이 물어보면, 잘 대답해주시면 면접관들이 좋아 죽습니다 😄)
| private Instant createdAt; | ||
| private Instant updatedAt; |
There was a problem hiding this comment.
예림님이 알고 계신지 모르기 때문에 가상의 상황을 제공해볼게요
요구사항으로 만약에 A라는 채널을 하나 만들었고, 요구사항중에 채널 생성일로부터 1시간 후의 시간을 노출해야한다고 해봅시다.
Channel channel = new Channel(...Instant.now() , Instant.now());
// getter로 받은 lastSeenAt에 1시간 추가한 Instant를 생성
Instant requiredInstant= channel.getCreatedAt().plusSeconds(3600);
System.out.println(channel.getCreatedAt());
System.out.println(requiredInstant);각각 어떤 값이 나오는지 생각해보시는게 좋을 것 같아요!
왜 나왔는지 고민해보시고 아래 레퍼런스를 참고해보시길 바랍니다 :D
(실무에서 정말로 필요한 상황이 아니라면 Instant, Date는 잘 사용하지 않습니다)
| public static Channel createChannel(ChannelType channelType, String title, String description) { | ||
| return new Channel(channelType, title, description); |
There was a problem hiding this comment.
👍
자, 사실 정적 팩토리 메소드를 쓰셨다면 빌더 패턴과 함께 쓰면 더 맛있을 것 같지 않나요😋?
정적 팩토리 메소드를 사용하면 받아야 하는 객체의 필드가 줄어들거나 많아진다면 그만큼의 생성자를 작성하고 호출을 해야 할 것 같아요. 그러면 코드도 점점 더 난잡해지겠죠?
@Builder(access = AccessLevel.PRIVATE)
public class Channel implements Serializable {
...
public static Channel createByXXX(ChannelType type, String name, String description) {
return Channel.builder()
.type(type)
.name(name)
.description(description)
.build();
}
}이런 방식도 있다는 것을 알아가시면 좋을 것 같아요
| public void update(String newName, String newEmail, String newPassword) { | ||
| boolean isChanged = false; | ||
| if (newName != null && !newName.equals(this.name)) { | ||
| this.name = newName; | ||
| isChanged = true; | ||
| } | ||
| if (newEmail != null && !newEmail.equals(this.email)) { | ||
| this.email = newEmail; | ||
| isChanged = true; | ||
| } | ||
| if (newPassword != null && !newPassword.equals(this.password)) { | ||
| this.password = newPassword; | ||
| isChanged = true; | ||
| } | ||
|
|
||
| public void removeChannel(Channel removeChannel) { | ||
| channelList.remove(removeChannel); | ||
| if (isChanged) { | ||
| this.updatedAt = Instant.now(); | ||
| } |
There was a problem hiding this comment.
null 체크를 엔티티 즉 도메인 내에서 해야하는 이유가 따로 있을까요?
만약에 객체의 특정 타입으로 인해서 반드시 null로 세팅을 해야하는 거라면 예림님 방식을 채택할 것 같은데,
단순히 파라미터들이 null인지에 대해서 체크하는 거라면 저는 Validator에서 처리하거나 서비스 로직에서 처리할 것 같아요.
public void update(String newName, String newEmail, String newPassword) {
this.name = newName;
this.email = newEmail;
this.password = newPassword;
this.updatedAt= LocalDateTime.now();
}| } | ||
|
|
||
| public void updateStatus() { | ||
| Instant ValidTime = this.updatedAt.plusSeconds(ADDITIONAL_TIME_SECONDS); |
There was a problem hiding this comment.
변수명에 첫 글자는 소문자, 그 다음부터는 대문자를 사용해주시면 됩니다.
validTime은 어떠실까요?
매직넘버는 잘 제거해주셨군요 👍
| public Status getStatus() { | ||
| updateStatus(); | ||
| return this.status; | ||
| } | ||
|
|
||
| public void updateStatus() { | ||
| Instant ValidTime = this.updatedAt.plusSeconds(ADDITIONAL_TIME_SECONDS); | ||
| if (ValidTime.compareTo(Instant.now()) > 0) { | ||
| this.status = Status.ONLINE; | ||
| } else { | ||
| this.status = Status.OFFLINE; | ||
| } | ||
| } |
| BinaryContent createUserProfileFile(MultipartFile file, UUID userId); | ||
| BinaryContent createMessageFile(MultipartFile file, UUID messageId); | ||
| BinaryContent updateUserProfileFile(MultipartFile file, UUID userId); | ||
| BinaryContent findById(UUID id); | ||
| List<BinaryContent> findAllByIdIn(List<UUID> ids); | ||
| void deleteById(UUID id); | ||
| void deleteByUserId(UUID userId); | ||
| void deleteAllByMessageId(UUID messageId); |
There was a problem hiding this comment.
저도 사실 네이밍에 시간을 많이 투자하는 편입니다 ^-^;;
(가끔씩은 chatgpt한테 추천도 받습니다😢)
저는 좀 규칙이 있는 것 같아요
조회 : getByXXX
생성 : createByXXX
...
만약에 여러건이다 하면
getAllByXXX
등등 쓰는 것 같아요.
확실한건 예를 들어 ContentService라고 했을 때,
get**Content**ByXXX
해당 클래스의 도메인은 중복으로 하지 않는 것 같아요.
또한, 엔티티에서 id값(pk든, fk든)을 여러개 들고 있다면 파라미터로 추상화를 풀어주는 것 같아요
ex)
getById(Long Id) // 도메인이 content이니깐 contentId는 사용 X
getByparentId(Long parentId) // 같은 파라미터일 때는 오버로딩이 안되기 때문에
뭐 이런식으로 하는 것 같아요!
| private ParentType parentType; | ||
| private UUID userId; | ||
| private UUID messageId; |
There was a problem hiding this comment.
저라면 parentId값만 가지고 올 것 같아요 :D 필요할때마다 즉각즉각 조회하면 되니깐요
(1~2번 추가 쿼리는 성능적으로 큰 영향이 없습니다, 오히려 다른 부분에 더 많은 영향을 끼치기 때문에)
만약에, parentId의 type이 변경이 된다면 당연히 BinaryContent도 조회를 하고 변경을 해줘야 하기 때문에 더 신경써야 하는 부분이 많지 않을까요
| private final UserStatusRepository userStatusRepository; | ||
| // private final UserService userService; 순환 참조 발생 |
There was a problem hiding this comment.
이것도 팀바팀, 회바회입니다. 이건 멘토링 시간에 다같이 들으면 좋을 것 같으니 그때 이야기해봐요 :D
요구사항
기본요구사항
Spring 프로젝트 초기화
3.4.0입니다.com.sprint.mission입니다.discodeit입니다.Jar입니다application.properties파일을yaml형식으로 변경하세요.DiscodeitApplication의 main 메서드를 실행하고 로그를 확인해보세요.Bean 선언 및 테스트
JavaApplication에서 테스트했던 코드를DiscodeitApplication에서 테스트해보세요.JavaApplication의 main 메소드를 제외한 모든 메소드를DiscodeitApplication클래스로 복사하세요.JavaApplication의 main 메소드에서 Service를 초기화하는 코드를 Spring Context를 활용하여 대체하세요.JavaApplication의 main 메소드의 셋업, 테스트 부분의 코드를DiscodeitApplication클래스로 복사하세요.Lombok 적용
@Getter로 대체해보세요.Basic*Service의 생성자를@RequiredArgsConstructor로 대체해보세요.추가 기능 요구사항
시간 타입 변경하기
Instant로 통일합니다.새로운 도메인 추가하기
id,createdAt,updatedAt)를 포함합니다.ReadStatusUserStatusBinaryContentupdatedAt필드는 정의하지 않습니다.User,Message도메인 모델과의 의존 관계 방향성을 잘 고려하여id참조 필드를 추가하세요.UserService 고도화
createusername과email은 다른 유저와 같으면 안됩니다.UserStatus를 같이 생성합니다.find,findAllupdatedeleteBinaryContent(프로필),UserStatusAuthService 구현
loginusername,password과 일치하는 유저가 있는지 확인합니다.ChannelService 고도화
createUser의 정보를 받아User별ReadStatus정보를 생성합니다.name과description속성은 생략합니다.findUser의id정보를 포함합니다.findAllUser의id정보를 포함합니다.User가 볼 수 있는 Channel 목록을 조회하도록 조회 조건을 추가하고, 메소드 명을 변경합니다.findAllByUserIdUser가 참여한 채널만 조회합니다.updatedeleteMessage,ReadStatusMessageService 고도화
createfindAllChannel의 Message 목록을 조회하도록 조회 조건을 추가하고, 메소드 명을 변경합니다.findallByChannelIdupdatedeleteBinaryContent)ReadStatusService 구현
createChannel이나User가 존재하지 않으면 예외를 발생시킵니다.Channel과User와 관련된 객체가 이미 존재하면 예외를 발생시킵니다.findid로 조회합니다.findAllByUserIduserId를 조건으로 조회합니다.updateid파라미터, 수정할 값 파라미터deleteid로 삭제합니다.UserStatusService 고도화
createUser가 존재하지 않으면 예외를 발생시킵니다.User와 관련된 객체가 이미 존재하면 예외를 발생시킵니다.findid로 조회합니다.findAllupdateid파라미터, 수정할 값 파라미터updateByUserIduserId로 특정User의 객체를 업데이트합니다.deleteid로 삭제합니다.BinaryContentService 구현
createfindid로 조회합니다.findAllByIdInid목록으로 조회합니다.deleteid로 삭제합니다.새로운 도메인 Repository 구현체 구현
심화
심화 요구사항
Bean 다루기
discodeit.repository.type설정값에 따라 Repository 구현체가 정해집니다.jcf이거나 없으면 JCF*Repository 구현체가 Bean으로 등록되어야 합니다.file이면 File*Repository 구현체가 Bean으로 등록되어야 합니다.주요 변경사항
스크린샷
멘토에게