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

Feature/opixxx step3 #27

Merged
merged 16 commits into from
Feb 17, 2025
Merged

Feature/opixxx step3 #27

merged 16 commits into from
Feb 17, 2025

Conversation

opixxx
Copy link

@opixxx opixxx commented Feb 10, 2025

정산 데이터 처리

  • 정산 요청 정보를 저장하기 위해서 Settlement(정산 요청 테이블), SettlementDetail(정산 세부 테이블)로 구성하였습니다.
  • Settlement, SettlementDetail의 생명주기가 동일하다고 생각하여 1:N 양방향 연관관계로 설정하였습니다.

정산 기능

  • 1 / N 정산 (4명이서 30006원을 정산한다고 가정)
    • 금액은 1 / N을 하게되면 7501원씩 정산하고, 나머지 2원이 남게된다. 그럼 남은 2원을 앞에서부터 1원씩 더 정산하도록 하고, 컬렉션을 무작위로 섞어 랜덤하게 추가 부담하도록 했습니다.
  • 랜덤 정산 (4명이서 정산한다고 가정)
    • N - 1명에게 10원 단위로 랜덤하게 분배한다. 분배하고 남은 금액을 나머지 한명이 부담하도록 하였습니다.

  • 정산 데이터가 많아질 경우를 생각하면 페이징 처리하는 방법을 추가 구현해야 합니다.
  • 정산 완료 처리를 어떻게 다룰지 고민 중 입니다.

- 1 / N 정산 : 1원 단위까지 동등하게 나눈 후 나머지 금액은 앞에서부터 추가해서 정산함
- 랜덤 정산 : 10원 단위로 N - 1 명에게 랜덤하게 정산 후 남은 금액을 나머지 한 명에게 정산
- 정산을 요청한 것을 조회 (내가 받아야 할 돈)
- 정산을 요청 받은 것을 조회 (내가 보내야 할 돈)
- 정산을 요청한 것을 조회 (내가 받아야 할 돈)
- 정산을 요청 받은 것을 조회 (내가 보내야 할 돈)
- SettlementDetail에는 정산을 요청 받은 사람들에 데이터만 저장
- 정산을 요청한 사람에 데이터는 Settlement에 저장
- 1/N 정산 기능 변경
  - 정산 후 앞에서 부터 남은 금액을 +1원씩 더해준후 Collections.shuffle()을 한다.
- getSettlement -> getRequestedSettlements
- 랜덤 정산이 동시에 요청이 들어올 경우 여러 쓰레드가 동시에 경합 - 패배 - 재도전을 반복하며 성능상의 문제 발생
- ThreadLocalRandom을 사용해서 각 쓰레드마다 생성된 인스턴스가 각각 난수를 반환하도록 하여 경합 문제가 발생하지 않아 성능상 이점이 있다.
- 랜덤 정산이 동시에 요청이 들어올 경우 여러 쓰레드가 동시에 경합 - 패배 - 재도전을 반복하며 성능상의 문제 발생
- ThreadLocalRandom을 사용해서 각 쓰레드마다 생성된 인스턴스가 각각 난수를 반환하도록 하여 경합 문제가 발생하지 않아 성능상 이점이 있다.
- ThreadLocalRandom도 seed를 알고 있으면 값을 예측할 수 있는 보안 문제가 있어 SecureRandom으로 변경
Copy link

@ZZAMBAs ZZAMBAs left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 기획에 따라 달라지는 단계이고 구현 자체는 단순하다보니 따로 리뷰 드릴 게 없었던 것 같아요.


assertThat(findSettlementDetail.get(0).getSettlement())
.isNotNull()
.extracting("id", "requestAccountId", "totalAmount", "type")
Copy link

Choose a reason for hiding this comment

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

여러 필드를 한번에 검증하기 좋은 메서드네요.

Copy link

@nyh365 nyh365 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~


@PositiveOrZero
@Max(value = 100, message = "100명까지 정산이 가능합니다.")
int totalNumber,
Copy link

Choose a reason for hiding this comment

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

totalNumber가 0이라면 이후 정산 금액을 계산하는 과정(getRandomSettlement, getEquallySettlement)에서 의도하지 않은 결과나 오류가 발생할 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

그렇겠네요 @min으로 최소 정산 인원을 검증하도록 변경하겠습니다


@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class SettlementUtil {
private static final SecureRandom SECURE_RANDOM = new SecureRandom();
Copy link

Choose a reason for hiding this comment

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

배워갑니다~👍

Copy link

@hellozo0 hellozo0 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 랜덤을 어떻게 나눠줄것인지 기준만 잡히면 구현만 하면되는 거라서 bb 테스트코드 정말 열심히 작성해주셨네요..! 저도 step들 다 끝나고 하나씩 적용해보겠습니다!

@opixxx opixxx merged commit ee5f3b5 into base/opixxx Feb 17, 2025
3 checks passed
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.

4 participants