-
Notifications
You must be signed in to change notification settings - Fork 24
[박진솔] sprint3 #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 박진솔
Are you sure you want to change the base?
The head ref may contain hidden characters: "\uBC15\uC9C4\uC194-sprint2"
[박진솔] sprint3 #90
Conversation
UUID userId, | ||
String channalName, | ||
List<UUID> entry, | ||
boolean isPriavate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오타있습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 그리고 이미 PrivateChannelCreateDTO
이냐 PublicChannelCreateDTO
에 따라서 어차피 로직이 다르게 동작할것 같은데 isPrivate
필드가 굳이 필요한지 저는 잘 모르겠어요. 있어도 상관은 없는데, 저는 안 썼을거 같아요. 이미 DTO 이름에서 의미가 명확하게 들어나서요.
public UserStatus(UUID userId) { | ||
this.id = UUID.randomUUID(); | ||
this.userId = userId; | ||
this.isLogin = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 코드 작성 의도가 궁금해서 여쭤보는겁니다.
UserStatus
항상 로그인 된 User가 호출할때만 생성되는 객체인가요?
User 입장에서 이미 로그인 했으면 login이 유지된 상태를 전제로 동작할거 같아요. 그래서 isLogin 필드는 굳이 필요없을거 같긴합니다.
|
||
Message save(Message message); | ||
List<Message> findAll(); | ||
List<Message> findAllFromChannel(UUID channel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findAllByChannel(UUID channelId)는 어떠신가요?
boolean existsId(UUID id); | ||
boolean existsUsername(String username); | ||
boolean existsEmail(String email); | ||
boolean existsName(String name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
18~ 21 라인들에 전부 중간에 By 넣어주면 좋을거 같아요.
|
||
// 아이디, 이메일 중복체크 | ||
public boolean isDuplicated(String args) { | ||
return userRepository.findAll().stream() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 유저를 다 가져와서 찾는건 너무 비효율적인거 같아요.
repository 레벨에서 username이랑 email로 동일한 유저가 있는지 조회하는 코드로 바꿔주실 수 있을까요?
이게 진솔님은 조금 낯서실수 있는데, RDB에서 특정 컬럼만 가지고 조회가 가능해요.
SELECT * FROM users WHERE username = '유저네임'
SELECT * FROM users WHERE email = '[email protected]'
그래서 이렇게 repository 에서 조건 걸어서 조회가 가능하다고 가정하시고 코드 작성해주시면 좋을거 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (userRepository.existsId(id)) { | ||
userRepository.delete(id); | ||
|
||
// 관련 도메인 삭제 | ||
binaryContentRepository.deleteByUserId(id); | ||
userStatusRepository.deleteByUserId(id); | ||
|
||
} else if (!userRepository.existsId(id)) { | ||
throw new NoSuchElementException("해당 사용자가 존재하지 않습니다."); | ||
} else { | ||
throw new IllegalArgumentException("잘못된 접근입니다."); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 if-else가 자주 나오는 형태가 가독성이 떨어지는 문제가 있습니다.
early return 이라는게 있어요. early return이 항상 좋은건 아닌데, 지금같은 상황에선 좋은거 같아요. 시간 되실때 한번 살펴보시겠어요?
참고 : https://woonys.tistory.com/209
아 그리고 154 번째 라인에서 userRepository.existsId(id)
값이 true인지 false인지 판단했잖아요. 그러면 161번째 라인에서 다시 userRepository.existsId(id)를 호출할 필요는 없어보여요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
early return으로 코드를 작성하면
if (!userRepository.existsId(id)) {
throw new NoSuchElementException("해당 사용자가 존재하지 않습니다.");
}
userRepository.delete(id);
// 관련 도메인 삭제
binaryContentRepository.deleteByUserId(id);
userStatusRepository.deleteByUserId(id);
PublicChannelCreateDTO publicChannelCreateDTO = new PublicChannelCreateDTO(user1.getId(), "첫번째 채팅방", false); | ||
PrivateChannelCreateDTO privateChannelCreateDTO = new PrivateChannelCreateDTO(user1.getId(), "두번째 채팅방", null, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 프로젝트에서 lombok을 사용하고 있는데, 기왕 롬복을 사용할거라면 인스턴스를 생성할때도 Builder 패턴을 사용하길 권장드려요.
이유는 new 생성자로 생성하는건 파라미터의 순서와 갯수를 매번 고려해야하고 읽는 사람이 가독성이 떨어지는 단점이 있어요.
반면에 Builder 패턴은 어떤 변수에 어떤 값이 들어갈지 바로 보이기 때문에 가독성이나 순서와 갯수를 고려하지 않아도 돼죠.
현대화된 언어들에서는Named Parameter
라는 개념으로 파라미터를 호출할때 지정하는 방법이 있는데, 자바에서는 아직 그게 없네요 ㅎㅎ 그래서 builder 패턴을 사용하는거에요.
참고
빌터패턴 : https://gwonbookcase.tistory.com/100?category=777301
Dart의 Named parameter : https://leftday.tistory.com/120
private final UUID id; | ||
private String name; | ||
private final UUID maker; | ||
private final boolean isPrivate; | ||
private List<UUID> entry; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
entry라는 List 필드로 UserId 관리하지 말고
ReadStatus로 User랑 Channe의 관계를 표현하는게 더 좋을거 같아요.
|
||
} | ||
|
||
public boolean checkUser(UUID id) throws IllegalAccessException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 메서드는 언제 호출되는거에요?
고생하셨습니다. 진솔님, 코드 완료되면 저한테 코드 변경했다고 답글에 멘션 걸어서 한번만 알려주세요. |
Spring 프로젝트 초기화
Bean 선언 및 테스트
추가 기능 요구사항
시간 타입 변경하기
새로운 도메인 추가하기
8y8gqa7me-image.png
공통: 앞서 정의한 도메인 모델과 동일하게 공통 필드(id, createdAt, updatedAt)를 포함합니다.
ReadStatus
UserStatus
BinaryContent
각 도메인 모델 별 레포지토리 인터페이스를 선언하세요.
DTO 활용하기
UserService 고도화
고도화
create
find, findAll
DTO를 활용하여:
update
delete
jv2bxgd12-image.png
AuthService 구현
usf34n0k1-image.png
ChannelService 고도화
고도화
create
find
DTO를 활용하여:
findAll
DTO를 활용하여:
findAllByUserId
update
delete
Message, ReadStatus
5f221mj11-image.png
MessageService 고도화
고도화
create
findAll
update
delete
3lpcv58wo-image.png
ReadStatusService 구현
create
find
findAllByUserId
update
delete
dvbyymgav-image.png
UserStatusService 고도화
create
find
findAll
update
updateByUserId
delete
wms3box4u-image.png
BinaryContentService 구현
create
find
findAllByIdIn
delete
1vjxng9jx-image.png
새로운 도메인 Repository 구현체 구현
w9rzlpt8k-image.png
멘토님께