Skip to content
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

InventoryCommandRepository에 amount 양수 검증 추가 #138

Merged

Conversation

daadaadaah
Copy link
Collaborator

@daadaadaah daadaadaah commented Jul 14, 2023

What

  • 아래 맥락들에서 서 나온 PR
  1. Redis 트랜잭션 대신 Redisson 분산락을 활용하여 재고 감소 로직 구현 #135 (comment)
  2. InventoryCommandRepository 추가 #56 (comment)

Comment

  1. 작업 우선순위에서는 후순위이지만, 다른 repositroy에도 특정 코너케이스들 처리해주는게 좋을까요? 아래의 경우를 예를 들면, getMany에서 Set<String> uuidStrings의 사이즈가 0일 때예요!
스크린샷 2023-07-14 오후 9 53 40
  1. 이렇게 코너 케이스가 있는 경우에는 단위테스트를 작성해두는게 좋겠죠?

@daadaadaah daadaadaah requested a review from f-lab-TJ July 14, 2023 13:01
@daadaadaah daadaadaah merged commit 38eba97 into f-lab-edu:develop Jul 14, 2023
1 check failed
@f-lab-TJ
Copy link

네, null이나 empty 체크는 기본으로 해주는게 좋을 것 같습니다.

String key = super.getRedisKey(dealProductUuid);

return (int) redisStringsRepository.decreaseByAmount(key, Long.valueOf(amount));
}

public void increaseByAmount(UUID dealProductUuid, int amount) {
validationAmountIsPositive(amount);

String key = super.getRedisKey(dealProductUuid);

redisStringsRepository.increaseByAmount(key, Long.valueOf(amount));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decrease가 감소 후 값을 반환해준다면 increase도 동일한 패턴으로 가는게 좋지않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재고 사후 검증 때문에 decrease는 return 값을 해주었는데, return 값을 사용하지 않아도 return 해주는게 좋을까요?
redis client에서도 @f-lab-TJ 코멘트처럼 둘다 return 값을 주긴합니다!ㅎㅎ

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

나중에 이 코드를 사용하는 사람이 덜 헷갈리는게 어떤 것일지, 정보의 소멸이 장점일지 단점일지 (Redis는 리턴 값을 줬으나 내 API는 생략하는 경우) 고민해보면 좋을 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#154 따로 PR 만들어서 수정했습니다~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants