내기 기능 구현 #589
내기 기능 구현 #589
Conversation
ay-eonii
requested review from
Mingyum-Kim,
ksk0605,
hoyeonyy and
pricelees
|
pr 제목 진짜 열받네요. |
|
안내면진다 목록 조회 ㅋㅋㅋㅋㅋㅋㅋㅋ |
| participant.getId(), | ||
| "" // TODO : profileUrl 추가 | ||
| ); |
There was a problem hiding this comment.
소셜 로그인 리팩토링 팀 완료되면 같이 이야기해봅시다!
| @Scheduled(fixedRateString = "${bet.schedule}") // 1분 | ||
| public void performScheduledTask() { | ||
| List<Bet> bets = betFinder.findAllDrawableBet(); | ||
|
|
||
| bets.forEach(Bet::draw); |
There was a problem hiding this comment.
fixedDelay는 이전 작업이 완료된 시점으로부터 주어진 시간 간격에 맞춰 작업을 실행하고
cron 표현식을 사용할 때는 항상 지정된 시간이 되면 항상 xx분 00초에 작동되는 방식이라고 합니다.
저희 도메인에는 cron 표현식이 더 적절한 듯해서 해당 부분 수정해야할 것 같아요!
| public BetFindAllResponses findAllBets(long darakbangId) { | ||
| List<Bet> bets = betFinder.findAllByDarakbangId(darakbangId); | ||
| return BetFindAllResponses.toResponse(bets); |
There was a problem hiding this comment.
정렬기준 구현도 필요하겠네요.
가장 마감시간이 임박한 순서대로 정렬기능 같이 구현해보죠!
| // TODO : 예외처리 공통화 | ||
| BetEntity betEntity = betRepository.findByIdAndDarakbangId(betId, darakbangId) | ||
| .orElseThrow(IllegalArgumentException::new); | ||
|
|
There was a problem hiding this comment.
예외 처리 공통화는 별도의 이슈를 파서 진행하도록 하시죠!
| private final BetFinder betFinder; | ||
| private final BetWriter betWriter; | ||
|
|
||
| @Scheduled(cron = "${bet.schedule}") // 1분 |
| List<DarakbangMember> darakbangMembers = betDarakbangMemberRepository.findAllDarakbangMemberByBetId( | ||
| betEntity.getId()); | ||
| List<Participant> participants = darakbangMembers.stream() | ||
| .map(darakbangMember -> new Participant(darakbangMember.getId(), darakbangMember.getNickname())) | ||
| .toList(); | ||
| return Bet.builder() | ||
| .betDetails(betEntity.toBetDetails()) | ||
| .moimerId(betEntity.getMoimerId()) | ||
| .loserId(betEntity.getLoserDarakbangMemberId()) | ||
| .participants(participants) | ||
| .build(); |
There was a problem hiding this comment.
오 맞습니다! 보니까 ParticiapantFinder의 로직과 겹치는 부분도 있어서 함께 분리해봤습니다.
| import mouda.backend.bet.domain.Bet; | ||
| import mouda.backend.bet.domain.BetDetails; | ||
|
|
||
| public record BetCreateRequest( |
There was a problem hiding this comment.
아직 valid 를 고려하지 않았어요. 예외처리 공통화와 함께 진행하려합니다. 꼼꼼한 리뷰 감사해요!
| public static BetEntity from(Bet bet) { | ||
| BetDetails betDetails = bet.getBetDetails(); | ||
|
|
||
| return BetEntity.builder() | ||
| .id(bet.getId()) | ||
| .title(betDetails.getTitle()) | ||
| .bettingTime(betDetails.getBettingTime()) | ||
| .loserDarakbangMemberId(bet.getLoserId()) | ||
| .moimerId(bet.getMoimerId()) | ||
| .build(); | ||
| } | ||
|
|
||
| public static BetEntity create(Bet bet, long darakbangId) { | ||
| return BetEntity.builder() | ||
| .title(bet.getBetDetails().getTitle()) | ||
| .bettingTime(bet.getBetDetails().getBettingTime()) | ||
| .moimerId(bet.getMoimerId()) | ||
| .darakbangId(darakbangId) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
도메인과 엔티티를 분리했는데, 이러면 엔티티에 어차피 도메인 의존이 생겨서 분리의 장점이 크게 느껴지지 않는 것 같아요. 차라리 별도의 팩토리로 분리하는 등 의존을 제거하면 좋겠음다
There was a problem hiding this comment.
개인적인 의견을 말씀드리면 엔티티와 도메인의 분리는 의존성 때문도 있지만 도메인 로직의 순수성을 지킨다는 목적에 더 가까워요.
기존처럼 도메인과 엔티티가 합쳐진 형태는 말 그대로
- 도메인 로직을 수행하는 역할
- DB의 데이터를 가져오는 역할
이 합쳐진 형태니 SRP를 위반한다 라고 볼 수도 있구요.
물론 상돌 말처럼 의존성의 관리도 매우 중요하다고 생각합니다!
다른 팀원들의 동의하에 Factory 혹은 Mapper 클래스를 도입해볼 수 있겠습니다.
패턴은 일관적이면 좋으니까요!
|
|
||
| public List<Bet> sort(List<Bet> bets) { | ||
| List<Bet> mutableBets = new ArrayList<>(bets); | ||
| LocalDateTime now = LocalDateTime.now().withSecond(0).withNano(0); |
There was a problem hiding this comment.
코드 전반적으로 withSecond, withNano가 중복되는데 이걸 한곳에 모아서 조금 더 의미있게 바꿀 수 있겠네요.
There was a problem hiding this comment.
BettingTime 객체로 추상화했어요! 좋은 리뷰 감사합니다.
PR의 목적이 무엇인가요?
안내면 진다 기능을 추가합니다.
API
이슈 ID는 무엇인가요?
설명
질문 혹은 공유 사항 (Optional)