-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Feature/27] 보틀 수락하기 API를 구현한다 #28
Conversation
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "user_a_id") | ||
val userA: User, | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "user_b_id") | ||
val userB: User, |
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.
수고하셨어요!
이번에는 미성님이 생각하는 구조가 궁금해서 질문 남겼어요!
val sourceUser = userRepository.findByIdOrNull(bottle.sourceUser.id) | ||
?: throw IllegalArgumentException("탈퇴한 회원이에요") | ||
|
||
return listOf(targetUser, sourceUser) |
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.
보틀을 저장할때, targetUser와 sourceUser를 나누는 기준은 어떤걸 생각하고 있으신가요?
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.
A와 B 유저가 있을 때,
A에게 B의 보틀이 오면 targetUser는 A, sourceUser는 B
라고 생각했어요!
그리고 A <- B 이렇게 와도 무조건 B <- A 가 성립하는 것은 아니라고 생각해서 저렇게 나눴어요!
저는 기획을 그렇게 이해했는데 인준님은 혹시 다르게 생각하시나요?
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.
보틀 매칭에 대해서는 정해진게 없어서 뭐 저희가 정하면 될거 같아요! ㅋㅋ 원래 회사였다면 당연히 기획을 받아와야겠지만 매칭은 일단 서버가 알아서 하기로 했으니..
A와 B 유저가 있을 때,
A에게 B의 보틀이 오면 targetUser는 A, sourceUser는 B
듣고나니 잘 이해됐어요 고맙습니다!
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "stopped_user_id") | ||
var stoppedUser: User? = null, | ||
|
||
@Column | ||
val status: PingPongStatus = PingPongStatus.ACTIVE, |
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.
PingPong 테이블이 유니크하게 가지는 정보는 stoppedUser/status인데, 요건 Bottle에 넣으면 PingPong 테이블이 없어도 될까요?
지금 핑퐁 중인 상태에서 조회하는 API를 치려면 User/Bottle/Letter/PingPong 최소 테이블 4개를 거쳐야해서 쪼끔 복잡도가 올라간 느낌이 있네요
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.
PingPong 테이블이 유니크하게 가지는 정보는 stoppedUser/status인데, 요건 Bottle에 넣으면 PingPong 테이블이 없어도 될까요?
그래도 되긴 하네요! 근데 저는 Bottle에 일정 시간 마다 새로운 데이터가 계속 추가되기 때문에 테이블 하나에 접근이 너무 몰린다고 생각해서 나눴어요! 그리고 Bottle과 PingPong은 성격이 조금 다르다고 생각했어요.
근데 그냥 성능상 문제가 있지 않을까 상상해본거라 저보다 경험이 많으신 인준님 의견에 따를게요! ㅋㅋㅋ 핑퐁 테이블은 없어도 될까요??
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.
아하 요거 어차피 하나의 db내여서 크게 성능차이는 없을거 같아요! ㅎㅎ 경험의 문제는 아니고 미성님은 어떻게 생각하고 설계하셨나 궁금했어요. 그럼 일단 심플하게 갈 수 있으면 심플하게 가시죠!
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.
좋아요 👍
@Column | ||
var image: String? = null, |
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.
이거 유저 사진이라면 두개가 필요하지 않나요? 그리고 imageUrl 이라는 변수면 좀 더 명확할거 같아요! binary 파일을 저장하는 경우도 있으니 구분되면 더 좋긴할거 같아요!
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.
아 하나의 보틀에 각각 letter가 생성되는걸 의도하신걸까요?
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.
네 맞아요! 누가 답변했는지, 누가 읽었는지를 하나의 letter에 표현할 수 없어서 각각 생성되도록 했어요.
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.
아하 네 이해했어요!
enum class PingPongStatus { | ||
ACTIVE, | ||
STOPPED, | ||
DONE, | ||
} |
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.
👍 저는 db에 상태값을 넣는게 처음 봤을때 신기했는데 미성님은 익숙하신거 같네요!
@Query( | ||
value = "SELECT * FROM Question " + | ||
"ORDER BY RAND() " + | ||
"LIMIT :numberOfQuestion", | ||
nativeQuery = true | ||
) | ||
fun findByRandom(@Param("numberOfQuestion") numberOfQuestion: Int): List<Question> |
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.
남겨주신 질문을 db에 남길지, 서버에서 갖고 있을지에 대해 제 생각은..!
둘다 괜찮을거 같아요! 이게 우리 서비스를 나타내는 데이터라면 db에 저장해서 보관하는게 맞는거 같고요. 회사에서는 퀴즈 이벤트를 한적이 있는데 db에 저장했었어요. 퀴즈 문제를 바꾸자고 서버 배포를 하는건 개발적 비용이 큰거 같아서요.
다만 매번 랜덤한 리스트를 조회하는건 db 찌르는 비용이 클거 같고, 캐싱을해서 서버에서 랜덤하게 주는게 좋을거 같아요!
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.
좋습니다!
그러면 데이터는 db에 저장하고 조회할 때는 모든 질문을 가져와서 서버에서 랜덤하게 뽑아서 주도록 할게요!
import javax.persistence.Converter | ||
|
||
@Converter(autoApply = true) | ||
class LetterQuestionAndAnswerConverter : AttributeConverter<List<LetterQuestionAndAnswer>, String> { |
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.
👍
@injoon2019 리뷰 반영했어요! |
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.
conflict 나는 것만 해결하고 merge 하면 될거 같아요 고생하셨어요!
val letters = questionRepository.findAll() | ||
.shuffled() | ||
.take(3) |
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.
좋습니다! CaffeineCache를 써보셨나요? 요건 나중에 캐시써서 개선하는걸로 해보시죠!
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.
아뇨 안써봤어요! 좋아요!!
package com.nexters.bottles.bottle.repository | ||
|
||
import com.nexters.bottles.pingpong.domain.Question | ||
import com.nexters.bottles.bottle.domain.Question |
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.
👍
💡 이슈 번호
close: #27
✨ 작업 내용
🚀 전달 사항