Skip to content

Refactor earnings calculations in WorkdayService and CompensationCalculator to use BigDecimal for precision and apply consistent rounding #45

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.

Sign up for GitHub

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

Merged
jeyongsong merged 2 commits into from
Apr 3, 2026
+49 −19
Merged

Refactor earnings calculations in WorkdayService and CompensationCalculator to use BigDecimal for precision and apply consistent rounding #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd5d810
Refactor earnings calculations in WorkdayService and CompensationCalc…
jeyongsong Apr 3, 2026
1df3762
Refactor earnings calculation logic in CompensationCalculator to hand…
jeyongsong Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/main/kotlin/com/moa/service/WorkdayService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.moa.service.notification.NotificationSyncService
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional
import java.math.BigDecimal
import java.math.RoundingMode
import java.time.LocalDate
import java.time.LocalTime
import java.time.YearMonth
Expand Down Expand Up @@ -77,7 +78,7 @@ class WorkdayService(
.findAllByMemberIdAndDateBetween(memberId, start, lastCalculableDate)
.associateBy { it.date }

var workedEarnings = 0L
var workedEarnings = BigDecimal.ZERO
var workedMinutes = 0L
var date = start
while (!date.isAfter(lastCalculableDate)) {
Expand All @@ -95,21 +96,21 @@ class WorkdayService(
completedWork.clockIn,
completedWork.clockOut,
)
workedEarnings += calculateDailyEarnings(
workedEarnings = workedEarnings.add(calculateDailyEarnings(
memberId,
date,
monthlyPolicy,
completedWork.type,
completedWork.clockIn,
completedWork.clockOut,
).toLong()
))
}

date = date.plusDays(1)
}

return MonthlyEarningsResponse(
workedEarnings = workedEarnings,
workedEarnings = workedEarnings.setScale(0, RoundingMode.HALF_UP).toLong(),
standardSalary = standardSalary,
workedMinutes = workedMinutes,
standardMinutes = standardMinutes,
Expand Down Expand Up @@ -389,7 +390,7 @@ class WorkdayService(

return calculateDailyEarnings(
memberId, date, policy, schedule.type, schedule.clockIn, schedule.clockOut,
).toInt()
).setScale(0, RoundingMode.HALF_UP).toInt()
}

/**
Expand Down
11 changes: 8 additions & 3 deletions src/main/kotlin/com/moa/service/calculator/CompensationCalculator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import java.time.*
*/
@Service
class CompensationCalculator {
companion object {
private const val MONEY_SCALE = 10
}

/**
* 주어진 기간의 기준 근무 시간을 계산합니다.
*
Expand Down Expand Up @@ -151,7 +155,7 @@ class CompensationCalculator {

if (workDaysCount == 0) return BigDecimal.ZERO

return monthlySalary.divide(BigDecimal(workDaysCount), 0, RoundingMode.HALF_UP)
return monthlySalary.divide(BigDecimal(workDaysCount), MONEY_SCALE, RoundingMode.HALF_UP)
}

/**
Expand All @@ -171,8 +175,9 @@ class CompensationCalculator {
actualWorkMinutes: Long,
): BigDecimal {
if (policyWorkMinutes <= 0) return dailyRate
val minuteRate = dailyRate.divide(BigDecimal(policyWorkMinutes), 10, RoundingMode.HALF_UP)
return minuteRate.multiply(BigDecimal(actualWorkMinutes)).setScale(0, RoundingMode.HALF_UP)
if (actualWorkMinutes == policyWorkMinutes) return dailyRate
val minuteRate = dailyRate.divide(BigDecimal(policyWorkMinutes), MONEY_SCALE, RoundingMode.HALF_UP)
return minuteRate.multiply(BigDecimal(actualWorkMinutes))
Comment on lines 176 to +180
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

calculateEarnings returns dailyRate directly when actualWorkMinutes == policyWorkMinutes, but returns a multiplied value (with MONEY_SCALE decimals) otherwise. Because BigDecimal equality is scale-sensitive (e.g., 100000 != 100000.0000000000), this introduces inconsistent scales depending on the branch, which can cause subtle bugs if callers use equals/hashing or serialize values. Consider normalizing the scale of the returned value (e.g., always setScale(MONEY_SCALE, ...) or stripTrailingZeros() consistently) before returning.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@jeyongsong jeyongsong Apr 3, 2026

Choose a reason for hiding this comment

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

BigDecimal scale inconsistency point is valid in general, but in this flow values are normalized at response boundaries via setScale(0, HALF_UP), and comparisons use compareTo semantics. So it should not affect current behavior. I’d prefer to avoid additional intermediate normalization here because that would reintroduce rounding into the calculation path we just fixed.

}

/**
Expand Down
46 changes: 35 additions & 11 deletions src/test/kotlin/com/moa/service/calculator/CompensationCalculatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.moa.entity.Workday
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import java.math.BigDecimal
import java.math.RoundingMode
import java.time.DayOfWeek
import java.time.LocalDate
import java.time.LocalTime
Expand Down Expand Up @@ -86,7 +87,7 @@ class CompensationCalculatorTest {
}

@Test
fun `휴무이면 저장된 시간 기반으로 급여를 계산한다`() {
fun `휴무가 정책 근무시간과 같으면 기준 일급과 동일한 급여를 계산한다`() {
val result = compensationCalculator.calculateDailyEarnings(
date = DATE,
salaryType = SalaryInputType.MONTHLY,
Expand All @@ -97,7 +98,7 @@ class CompensationCalculatorTest {
clockOutTime = LocalTime.of(18, 0),
)

assertThat(result.toLong()).isEqualTo(142857L)
assertThat(result).isEqualByComparingTo(BigDecimal("142857.1428571429"))
}

@Test
Expand All @@ -116,7 +117,7 @@ class CompensationCalculatorTest {
}

@Test
fun `정상 근무시 일급을 반환한다`() {
fun `정상 근무시 기준 일급을 반환한다`() {
val result = compensationCalculator.calculateDailyEarnings(
date = DATE,
salaryType = SalaryInputType.MONTHLY,
Expand All @@ -127,7 +128,7 @@ class CompensationCalculatorTest {
clockOutTime = LocalTime.of(18, 0),
)

assertThat(result.toLong()).isEqualTo(142857L)
assertThat(result).isEqualByComparingTo(BigDecimal("142857.1428571429"))
}

@Test
Expand Down Expand Up @@ -189,7 +190,7 @@ class CompensationCalculatorTest {
clockOutTime = null,
)

assertThat(result.toLong()).isEqualTo(142857L)
assertThat(result).isEqualByComparingTo(BigDecimal("142857.1428571429"))
}

@Test
Expand All @@ -207,7 +208,7 @@ class CompensationCalculatorTest {
),
)

assertThat(result).isEqualByComparingTo(BigDecimal("142857"))
assertThat(result).isEqualByComparingTo(BigDecimal("142857.1428571429"))
}

@Test
Expand Down Expand Up @@ -235,11 +236,11 @@ class CompensationCalculatorTest {
),
)

assertThat(result).isEqualByComparingTo(BigDecimal("230769"))
assertThat(result).isEqualByComparingTo(BigDecimal("230769.2307692308"))
}

@Test
fun `일급 계산 결과의 소수점은 반올림한다`() {
fun `일급 계산 결과는 월 합산 전까지 소수점을 유지한다`() {
val result = compensationCalculator.calculateDailyRate(
targetDate = LocalDate.of(2025, 6, 1),
salaryType = SalaryInputType.MONTHLY,
Expand All @@ -251,7 +252,7 @@ class CompensationCalculatorTest {
),
)

assertThat(result.scale()).isEqualTo(0)
assertThat(result.scale()).isGreaterThan(0)
}

@Test
Expand Down Expand Up @@ -299,7 +300,7 @@ class CompensationCalculatorTest {

val result = compensationCalculator.calculateEarnings(dailyRate, 540, 600)

assertThat(result).isEqualByComparingTo(BigDecimal("111111"))
assertThat(result).isEqualByComparingTo(BigDecimal("111111.1111111200"))
}

@Test
Expand All @@ -308,7 +309,30 @@ class CompensationCalculatorTest {

val result = compensationCalculator.calculateEarnings(dailyRate, 540, 480)

assertThat(result).isEqualByComparingTo(BigDecimal("88889"))
assertThat(result).isEqualByComparingTo(BigDecimal("88888.8888888960"))
}

@Test
fun `월급을 일별 반올림하지 않고 월 합계에서만 반올림하면 월 총액이 유지된다`() {
val dailyRate = compensationCalculator.calculateDailyRate(
targetDate = LocalDate.of(2025, 6, 1),
salaryType = SalaryInputType.MONTHLY,
salaryAmount = 3_000_000,
workDays = setOf(
DayOfWeek.MONDAY,
DayOfWeek.TUESDAY,
DayOfWeek.WEDNESDAY,
DayOfWeek.THURSDAY,
DayOfWeek.FRIDAY,
),
)

val monthlyTotal = (1..21).fold(BigDecimal.ZERO) { acc, _ ->
acc.add(compensationCalculator.calculateEarnings(dailyRate, 540, 540))
}

assertThat(monthlyTotal.setScale(0, RoundingMode.HALF_UP))
.isEqualByComparingTo(BigDecimal("3000000"))
}

@Test
Expand Down
Loading