[BE REFACTOR: ItemHistory에 amount 컬럼 추가#1695 #1696
Conversation
saewoo1
changed the title
| userIds.forEach(userId -> { | ||
| userCommandService.updateCoinAmount(userId, amount); | ||
| }); |
There was a problem hiding this comment.
userId 개수당 N회의 쿼리일 것 같은데 in 절을 이용해서 updateMany로 처리하시는 건 어떤가요??
라고 쓰고보니 JPA여서 commit할 때 모아서 알아서 해줬던가..
There was a problem hiding this comment.
update는 모아서 처리해주지 않습니다. 전부 동일한 상태로의 변경이라면 @Modifying 애노테이션을 활용해서 bulk update를 수행하는게 좋을 것 같습니다~
| userIds.forEach(userId -> { | ||
| userCommandService.updateCoinAmount(userId, amount); | ||
| }); |
There was a problem hiding this comment.
update는 모아서 처리해주지 않습니다. 전부 동일한 상태로의 변경이라면 @Modifying 애노테이션을 활용해서 bulk update를 수행하는게 좋을 것 같습니다~
| @@ -96,6 +105,15 @@ public static ItemHistory of(long userId, long itemId, LocalDateTime usedAt) { | |||
| return itemHistory; | |||
| } | |||
|
|
|||
| public static ItemHistory coin(Long userId, Long itemId, LocalDateTime assignedAt, | |||
There was a problem hiding this comment.
위의 생성자 팩토리 메서드의 메서드명으로 coin만 적어둔다면, 메서드명만 보고 생성자 팩토리 메서드일 것이다라고 유추하기는 힘들어 보입니다 .createHistoryWithCoin 등 조금 더 직관적인 이름이 좋을 것 같습니다
There was a problem hiding this comment.
혹시 제가 작성한 이슈에 대해 어떻게 생각하시나요?
일단은 amount 필드를 추가하는 방식으로 진행했지만 더 좋은 아이디어가 있을까 하고 생성자 부분은 모두 임시적으로 만들어둔거라서요..!
amount를 추가하는걸로 확정이 된다면 생성자 관련 처리나 sku 통합(기존 관리자 지급 코인 100, 200으로 명시해둔 부분)을 진행하려 합니다!
| Long amount) { | ||
| ItemHistory itemHistory = new ItemHistory(userId, itemId, assignedAt, amount); | ||
| if (!itemHistory.isValid()) { | ||
| throw ExceptionStatus.INVALID_ARGUMENT.asControllerException(); |
There was a problem hiding this comment.
ControllerException이 아닌 DomainException이 적합해 보입니다.
There was a problem hiding this comment.
어흑 마이깟 오류 발견 감사합니다
| @@ -96,6 +105,15 @@ public static ItemHistory of(long userId, long itemId, LocalDateTime usedAt) { | |||
| return itemHistory; | |||
| } | |||
There was a problem hiding this comment.
amount 필드가 추가 되었음에 따라 기존의 amount를 파라미터로 받지 않는 생성자의 경우 해당 값에 대한 처리를 해주는 것이 좋아보입니다. 해당 내용을 모르고 이 팩토리 메서드를 사용한다면, 참조 타입이기 때문에 amount에 null이 저장되어 런타임에 DB에 저장이 안되는 에러가 발생할 것 같습니다
…/innovationacademy-kr/Cabi into be/dev/refactor-ItemHistory#1695
…/innovationacademy-kr/Cabi into be/dev/refactor-ItemHistory#1695
해당 사항 (중복 선택)
설명
#1695
반드시 위 이슈를 읽어주십셔 ItemHistory에 amount 컬럼이 추가되는게 최선일까여?????
더 좋은 아이디어가 있다면 편하게 말씀해주세여!!!!