-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: 마이페이지 카페 조회 시 roadAddress 및 내가 남긴 리뷰 함께 조회 기능 구현 #137
Conversation
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.
코드 로직 자체는 문제 없어보여서 approve 할게요. 수고하셨습니다~ 😀
|
||
if (cafeOptional.isPresent()) { | ||
cafeOptional.get().updateCafeRoadAddress(request.getRoadAddress()); | ||
return; |
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문을 아래와 같이 람다식으로 표현할 수 있을 거 같은데 어떠신가요?? 아 생각을 해보니 이미 존재하는 카페에 대해서는 return을 해줘야 하니 원래 코드로 가야할 거 같습니다 ㅎㅎ..
cafeOptional.ifPresent(existingCafe -> {
existingCafe.updateCafeRoadAddress(request.getRoadAddress());
});
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.
람다식을 활용한다면 ifPresentOrElse
도 좋은 선택지일 듯 해요.
@Transactional
public void save(CafeRegisterRequest request) {
Cafe cafe = new Cafe(request.getId(), request.getName(), request.getRoadAddress());
try {
cafeRepository.findByMapId(request.getId())
.ifPresentOrElse(
existedCafe -> existedCafe.updateCafeRoadAddress(request.getRoadAddress()),
() -> cafeRepository.save(cafe)
);
} catch (DataIntegrityViolationException e) {
throw new DuplicateCafeException();
}
}
람다식을 활용하지 않는다면 59~64번째 줄을 메서드 분리하는 게 좋아보여요.
처음에는 무슨 코드인가 했어서요..ㅎㅎ
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.
이해하기 편하게 하려고 함수형 보단 절차지향으로 짜봤는데, 오히려 독이 되었군요.. 고쳐서 반영해둘게요!
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 (cafeOptional.isPresent()) { | ||
cafeOptional.get().updateCafeRoadAddress(request.getRoadAddress()); | ||
return; |
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.
람다식을 활용한다면 ifPresentOrElse
도 좋은 선택지일 듯 해요.
@Transactional
public void save(CafeRegisterRequest request) {
Cafe cafe = new Cafe(request.getId(), request.getName(), request.getRoadAddress());
try {
cafeRepository.findByMapId(request.getId())
.ifPresentOrElse(
existedCafe -> existedCafe.updateCafeRoadAddress(request.getRoadAddress()),
() -> cafeRepository.save(cafe)
);
} catch (DataIntegrityViolationException e) {
throw new DuplicateCafeException();
}
}
람다식을 활용하지 않는다면 59~64번째 줄을 메서드 분리하는 게 좋아보여요.
처음에는 무슨 코드인가 했어서요..ㅎㅎ
comment.getCafe().getMapId(), | ||
comment.getCafe().getName(), | ||
comment.getCafe().getStudyType(), | ||
comment.getContent() | ||
comment.getContent(), | ||
comment.getCafe().getRoadAddress() |
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.
코멘트가 작성된 카페의 정보에 대한 response를 보내줘야 할 때가 좀 있네요.
아예 DTO에 Cafe 엔티티를 받는 정적 팩터리 메서드를 만드는 것도 방법일 거 같기도 하고요...
comment의 getter + getter 디미터 법칙이 조금 걸려서 comment 엔티티에 도로명주소 반환해주는 메서드를 만들어도 될 거 같기도 하고.
조금 고민되네요. 한번 같이 생각해주시고 적절하게 반영해주세요!
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.
요즘 코틀린 쓰다보니까 너무 생각없이 디미터 법칙을 무시해버렸는데, 위 경우 get -> get으로 되는 경우가 ㅁ많아서 comment 엔티티에 메서드가 너무 많이 생길 수도 있을 것 같아서 우선은 from 정적 팩토리 메서드 패턴을 쓰는게 나아보여서 그렇게 변경했습니다!
private String roadAddress; | ||
private String wifi; | ||
private String parking; | ||
private String toilet; | ||
private String power; | ||
private String sound; | ||
private String desk; |
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.
responseBody에 roadAress / 모든 리뷰(스터디타입, 와이파이 등등) 추가
요런 요구사항이 있었어서 우선 내가 쓴 리뷰의 Cafe Detail로 반영했습니다!
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.
리뷰 반영된 내용 확인했습니다!
public static MyCommentCafeResponse from(Comment comment) { | ||
return new MyCommentCafeResponse( | ||
comment.getCafe().getMapId(), | ||
comment.getCafe().getName(), | ||
comment.getCafe().getStudyType(), | ||
comment.getContent(), | ||
comment.getCafe().getRoadAddress() | ||
); | ||
} |
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.
요런 정적팩터리메서드를 생각하긴 했는데요
MyCommentCafeResponse of(String content, Cafe commentCafe)
getter + getter를 좀 줄일만한 그런 느낌으로?
근데 어느게 나은진 잘 모르겠네요 😅
개요
작업사항
주의사항