Conversation
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 파티 분배금 계산 로직을 수정하여 송금 수수료를 고려하도록 변경하고, 이로 인해 발생했던 계산 오류를 해결합니다. 새로운 로직은 모든 파티원이 설정된 비율에 맞게 최종 금액을 수령할 수 있도록 하는 '공정한 기준 이익' 개념을 도입하여 문제를 해결합니다. 전반적으로 좋은 방향의 수정이지만, distributeProfitByPercent 유틸리티 함수에서 발견된 몇 가지 잠재적인 문제점(치명적인 버그 포함)과 설계 개선점에 대한 피드백을 남깁니다.
| export const distributeProfitByPercent = ( | ||
| totalProfit: number, | ||
| feeRate: number, | ||
| members: Member[], | ||
| mode: Mode, | ||
| ) => { | ||
| const memberCount = members.length | ||
| ): DistributionResult[] => { | ||
| const normalizedMembers = normalizeMembers(members, mode) | ||
|
|
||
| const fee = memberCount >= 2 ? Math.floor((totalProfit * feeRate) / 100) : 0 | ||
| const distributable = totalProfit - fee | ||
| const activeOwnerIndex = 0 | ||
| const ownerMember = normalizedMembers[activeOwnerIndex] | ||
|
|
||
| const results: DistributionResult[] = members.map((m) => ({ | ||
| ...m, | ||
| amount: | ||
| mode === 'MANUAL' | ||
| ? Math.floor((distributable * m.ratio) / 100) | ||
| : Math.floor(distributable / memberCount), | ||
| })) | ||
| const fairBaseProfit = calculateFairBaseProfit( | ||
| totalProfit, | ||
| ownerMember.ratio, | ||
| feeRate, | ||
| ) | ||
|
|
||
| const distributed = results.reduce((sum, r) => sum + r.amount, 0) | ||
| const remainder = distributable - distributed | ||
| const results = normalizedMembers.map((member, idx) => | ||
| computeMemberShare( | ||
| member, | ||
| fairBaseProfit, | ||
| feeRate, | ||
| idx === activeOwnerIndex, // isOwner check | ||
| ), | ||
| ) | ||
|
|
||
| return { results, remainder } | ||
| return results | ||
| } |
There was a problem hiding this comment.
이 함수는 더 단순하고 견고하게 리팩토링할 수 있습니다. 현재 normalizeMembers 함수에 의존하고 있는데, 이 함수는 상위 훅의 로직을 중복 구현하고 있으며 빈 멤버 배열이 전달될 경우 크래시를 유발할 수 있습니다. 또한, 함수가 불필요하게 mode에 의존하고 있습니다.
mode와 normalizeMembers에 대한 의존성을 제거하여 이 함수를 순수한 계산 유틸리티로 만들고, 상위 훅이 멤버 비율을 설정하도록 책임을 위임하는 것이 좋습니다. 이렇게 하면 관심사가 명확히 분리되고 잠재적인 크래시를 해결할 수 있습니다. 아래 제안을 적용한 후에는 normalizeMembers 함수를 삭제하고, usePartyDistribution.ts에서 distributeProfitByPercent를 호출하는 부분도 mode를 전달하지 않도록 수정해야 합니다.
export const distributeProfitByPercent = (
totalProfit: number,
feeRate: number,
members: Member[],
): DistributionResult[] => {
if (members.length === 0) {
return [];
}
const activeOwnerIndex = 0;
const ownerMember = members[activeOwnerIndex];
const fairBaseProfit = calculateFairBaseProfit(
totalProfit,
ownerMember.ratio,
feeRate,
);
const results = members.map((member, idx) =>
computeMemberShare(
member,
fairBaseProfit,
feeRate,
idx === activeOwnerIndex, // isOwner check
),
);
return results;
};| const activeOwnerIndex = 0 | ||
| const ownerMember = normalizedMembers[activeOwnerIndex] |
There was a problem hiding this comment.
분배 주체인 파티장이 항상 배열의 첫 번째(index 0) 멤버라고 가정하고 있습니다. 이는 유연하지 못한 설계이며, 향후 멤버 순서를 변경하거나 다른 파티장을 지정하는 기능이 추가될 경우 버그를 유발할 수 있습니다. 파티장의 ID를 함수에 전달하거나 Member 타입에 isOwner 같은 플래그를 추가하여 명시적으로 파티장을 식별하는 것이 더 견고한 접근 방식입니다.
1 participant
개요
경매장 판매 후 실수령액 기준으로 분배하는 과정에서 팀원 송금 시 발생하는 수수료를 고려하지 않아
분배 비율을 수동으로 변경할 경우 계산 결과가 잘못되는 문제를 수정하고 송금해야 하는 금액을 추가함
구조 변경
normalizeMembers
calculateFairBaseProfit
computeMemberShare