Skip to content

Commit 1f34381

Browse files
adamsaghyadamsaghy
committed
FINERACT-2326: Fix rounding of BigDecimal when added to Money object
1 parent da1f310 commit 1f34381

File tree

26 files changed

+147
-276
lines changed

26 files changed

+147
-276
lines changed

‎fineract-core/src/main/java/org/apache/fineract/organisation/monetary/domain/Money.java‎

Lines changed: 54 additions & 184 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@
2020

2121
import java.math.BigDecimal;
2222
import java.math.MathContext;
23-
import java.util.Iterator;
23+
import lombok.Getter;
2424
import org.apache.fineract.infrastructure.core.service.MathUtil;
2525
import org.apache.fineract.organisation.monetary.data.CurrencyData;
2626

2727
public class Money implements Comparable<Money> {
2828

29+
@Getter
2930
private final BigDecimal amount;
3031
private final CurrencyData currency;
3132

@@ -52,53 +53,6 @@ private Money(final CurrencyData currency, final BigDecimal amount, final MathCo
5253
this.amount = amountScaled.setScale(currency.getDecimalPlaces(), getMc().getRoundingMode());
5354
}
5455

55-
public MonetaryCurrency getCurrency() {
56-
return MonetaryCurrency.fromCurrencyData(currency);
57-
}
58-
59-
public CurrencyData getCurrencyData() {
60-
return currency;
61-
}
62-
63-
public String getCurrencyCode() {
64-
return currency.getCode();
65-
}
66-
67-
public Integer getInMultiplesOf() {
68-
return currency.getInMultiplesOf();
69-
}
70-
71-
public BigDecimal getAmount() {
72-
return amount;
73-
}
74-
75-
public BigDecimal getAmountDefaultedToNullIfZero() {
76-
return defaultToNullIfZero(this.amount);
77-
}
78-
79-
public static Money total(final Money... monies) {
80-
if (monies.length == 0) {
81-
throw new IllegalArgumentException("Money array must not be empty");
82-
}
83-
Money total = monies[0];
84-
for (int i = 1; i < monies.length; i++) {
85-
total = total.plus(monies[i]);
86-
}
87-
return total;
88-
}
89-
90-
public static Money total(final Iterable<? extends Money> monies) {
91-
final Iterator<? extends Money> it = monies.iterator();
92-
if (!it.hasNext()) {
93-
throw new IllegalArgumentException("Money iterator must not be empty");
94-
}
95-
Money total = it.next();
96-
while (it.hasNext()) {
97-
total = total.plus(it.next());
98-
}
99-
return total;
100-
}
101-
10256
public static Money of(final CurrencyData currency, final BigDecimal newAmount) {
10357
return of(currency, newAmount, MoneyHelper.getMathContext());
10458
}
@@ -131,22 +85,6 @@ public static Money zero(final CurrencyData currency) {
13185
return zero(currency, MoneyHelper.getMathContext());
13286
}
13387

134-
public static double roundToMultiplesOf(final double existingVal, final Integer inMultiplesOf) {
135-
double amountScaled;
136-
final double ceilingOfValue = ceiling(existingVal, inMultiplesOf);
137-
final double floorOfValue = floor(existingVal, inMultiplesOf);
138-
139-
final double floorDiff = existingVal - floorOfValue;
140-
final double ceilDiff = ceilingOfValue - existingVal;
141-
142-
if (ceilDiff > floorDiff) {
143-
amountScaled = floorOfValue;
144-
} else {
145-
amountScaled = ceilingOfValue;
146-
}
147-
return amountScaled;
148-
}
149-
15088
public static BigDecimal roundToMultiplesOf(final BigDecimal existingVal, final Integer inMultiplesOf) {
15189
BigDecimal amountScaled = existingVal;
15290
BigDecimal inMultiplesOfValue = BigDecimal.valueOf(inMultiplesOf);
@@ -169,68 +107,35 @@ public static Money roundToMultiplesOf(final Money existingVal, final Integer in
169107
return Money.of(existingVal.getCurrencyData(), amountScaled);
170108
}
171109

172-
public static double ceiling(final double n, final double s) {
173-
double c;
174-
175-
if ((n < 0 && s > 0) || (n > 0 && s < 0)) {
176-
c = Double.NaN;
177-
} else {
178-
c = (n == 0 || s == 0) ? 0 : Math.ceil(n / s) * s;
179-
}
180-
181-
return c;
182-
}
183-
184-
public static double floor(final double n, final double s) {
185-
double f;
186-
187-
if ((n < 0 && s > 0) || (n > 0 && s < 0) || (s == 0 && n != 0)) {
188-
f = Double.NaN;
189-
} else {
190-
f = n == 0 ? 0 : Math.floor(n / s) * s;
191-
}
192-
193-
return f;
194-
}
195-
196-
private static BigDecimal defaultToZeroIfNull(final BigDecimal value) {
197-
BigDecimal result = BigDecimal.ZERO;
198-
if (value != null) {
199-
result = value;
200-
}
201-
return result;
110+
public MonetaryCurrency getCurrency() {
111+
return MonetaryCurrency.fromCurrencyData(currency);
202112
}
203113

204-
private static BigDecimal defaultToNullIfZero(final BigDecimal value) {
205-
BigDecimal result = value;
206-
if (value != null && BigDecimal.ZERO.compareTo(value) == 0) {
207-
result = null;
208-
}
209-
return result;
114+
public CurrencyData getCurrencyData() {
115+
return currency;
210116
}
211117

212-
public Money copy() {
213-
return new Money(this.currency, this.amount, this.mc);
118+
public String getCurrencyCode() {
119+
return currency.getCode();
214120
}
215121

216-
public Money copy(final BigDecimal amount) {
217-
return new Money(this.currency, amount, this.mc);
122+
public Integer getInMultiplesOf() {
123+
return currency.getInMultiplesOf();
218124
}
219125

220-
public Money copy(final double amount) {
221-
return copy(BigDecimal.valueOf(amount));
126+
public BigDecimal getAmountDefaultedToNullIfZero() {
127+
return defaultToNullIfZero(this.amount);
222128
}
223129

224130
public Money plus(final Iterable<? extends Money> moniesToAdd) {
225-
BigDecimal total = this.amount;
131+
Money total = this;
226132
for (final Money moneyProvider : moniesToAdd) {
227133
if (moneyProvider == null) {
228134
continue;
229135
}
230-
final Money money = checkCurrencyEqual(moneyProvider);
231-
total = total.add(money.amount);
136+
total = total.plus(moneyProvider);
232137
}
233-
return Money.of(getCurrencyData(), total);
138+
return total;
234139
}
235140

236141
public Money plus(final Money moneyToAdd) {
@@ -254,18 +159,13 @@ public Money plus(final BigDecimal amountToAdd, MathContext mc) {
254159
if (amountToAdd == null || amountToAdd.compareTo(BigDecimal.ZERO) == 0) {
255160
return this;
256161
}
257-
final BigDecimal newAmount = this.amount.add(amountToAdd);
162+
// It's important to do the scaling and rounding on the original amount to avoid rounding issues in case of
163+
// rounding modes like HALF_EVEN
164+
Money amountToAddMoney = Money.of(getCurrencyData(), amountToAdd);
165+
final BigDecimal newAmount = this.amount.add(amountToAddMoney.getAmount());
258166
return Money.of(getCurrencyData(), newAmount, mc);
259167
}
260168

261-
public Money plus(final double amountToAdd) {
262-
if (amountToAdd == 0) {
263-
return this;
264-
}
265-
final BigDecimal newAmount = this.amount.add(BigDecimal.valueOf(amountToAdd));
266-
return Money.of(getCurrencyData(), newAmount);
267-
}
268-
269169
public Money minus(final Money moneyToSubtract) {
270170
return minus(moneyToSubtract, getMc());
271171
}
@@ -278,30 +178,6 @@ public Money minus(final Money moneyToSubtract, final MathContext mc) {
278178
return this.minus(toSubtract.getAmount(), mc);
279179
}
280180

281-
public Money add(final Money moneyToAdd) {
282-
return add(moneyToAdd, getMc());
283-
}
284-
285-
public Money add(final Money moneyToAdd, final MathContext mc) {
286-
if (moneyToAdd == null) {
287-
return this;
288-
}
289-
final Money toAdd = checkCurrencyEqual(moneyToAdd);
290-
return this.add(toAdd.getAmount(), mc);
291-
}
292-
293-
public Money add(final BigDecimal amountToAdd) {
294-
return add(amountToAdd, getMc());
295-
}
296-
297-
public Money add(final BigDecimal amountToAdd, final MathContext mc) {
298-
if (amountToAdd == null || amountToAdd.compareTo(BigDecimal.ZERO) == 0) {
299-
return this;
300-
}
301-
final BigDecimal newAmount = this.amount.add(amountToAdd);
302-
return Money.of(getCurrencyData(), newAmount, mc);
303-
}
304-
305181
public Money minus(final BigDecimal amountToSubtract) {
306182
return minus(amountToSubtract, getMc());
307183
}
@@ -310,21 +186,13 @@ public Money minus(final BigDecimal amountToSubtract, final MathContext mc) {
310186
if (amountToSubtract == null || amountToSubtract.compareTo(BigDecimal.ZERO) == 0) {
311187
return this;
312188
}
313-
final BigDecimal newAmount = this.amount.subtract(amountToSubtract);
189+
// It's important to do the scaling and rounding on the original amount to avoid rounding issues in case of
190+
// rounding modes like HALF_EVEN
191+
Money amountToSubtractMoney = Money.of(getCurrencyData(), amountToSubtract);
192+
final BigDecimal newAmount = this.amount.subtract(amountToSubtractMoney.getAmount());
314193
return Money.of(getCurrencyData(), newAmount, mc);
315194
}
316195

317-
private Money checkCurrencyEqual(final Money money) {
318-
if (!isSameCurrency(money)) {
319-
throw new UnsupportedOperationException("currencies are different.");
320-
}
321-
return money;
322-
}
323-
324-
public boolean isSameCurrency(final Money money) {
325-
return getCurrencyCode().equals(money.getCurrencyCode());
326-
}
327-
328196
public Money dividedBy(final BigDecimal valueToDivideBy, final MathContext mc) {
329197
if (valueToDivideBy.compareTo(BigDecimal.ONE) == 0) {
330198
return this;
@@ -333,14 +201,6 @@ public Money dividedBy(final BigDecimal valueToDivideBy, final MathContext mc) {
333201
return Money.of(getCurrencyData(), newAmount, mc);
334202
}
335203

336-
public Money dividedBy(final double valueToDivideBy, final MathContext mc) {
337-
if (valueToDivideBy == 1) {
338-
return this;
339-
}
340-
final BigDecimal newAmount = this.amount.divide(BigDecimal.valueOf(valueToDivideBy), mc);
341-
return Money.of(getCurrencyData(), newAmount, mc);
342-
}
343-
344204
public Money dividedBy(final long valueToDivideBy, final MathContext mc) {
345205
if (valueToDivideBy == 1) {
346206
return this;
@@ -369,14 +229,6 @@ public Money multipliedBy(final BigDecimal valueToMultiplyBy, final MathContext
369229
return Money.of(getCurrencyData(), newAmount, mc);
370230
}
371231

372-
public Money multipliedBy(final double valueToMultiplyBy) {
373-
if (valueToMultiplyBy == 1) {
374-
return this;
375-
}
376-
final BigDecimal newAmount = this.amount.multiply(BigDecimal.valueOf(valueToMultiplyBy));
377-
return Money.of(getCurrencyData(), newAmount);
378-
}
379-
380232
public Money multipliedBy(final long valueToMultiplyBy) {
381233
return multipliedBy(valueToMultiplyBy, getMc());
382234
}
@@ -389,19 +241,6 @@ public Money multipliedBy(final long valueToMultiplyBy, final MathContext mc) {
389241
return Money.of(getCurrencyData(), newAmount, mc);
390242
}
391243

392-
public Money multiplyRetainScale(final BigDecimal valueToMultiplyBy, final MathContext mc) {
393-
if (valueToMultiplyBy.compareTo(BigDecimal.ONE) == 0) {
394-
return this;
395-
}
396-
BigDecimal newAmount = this.amount.multiply(valueToMultiplyBy, mc);
397-
newAmount = newAmount.setScale(this.currency.getDecimalPlaces(), mc.getRoundingMode());
398-
return Money.of(getCurrencyData(), newAmount, mc);
399-
}
400-
401-
public Money multiplyRetainScale(final double valueToMultiplyBy, final MathContext mc) {
402-
return this.multiplyRetainScale(BigDecimal.valueOf(valueToMultiplyBy), mc);
403-
}
404-
405244
public Money percentageOf(BigDecimal percentage, final MathContext mc) {
406245
final BigDecimal newAmount = this.amount.multiply(percentage).divide(BigDecimal.valueOf(100), mc);
407246
return Money.of(getCurrencyData(), newAmount, mc);
@@ -439,6 +278,10 @@ public boolean isGreaterThan(final Money other) {
439278
return compareTo(other) > 0;
440279
}
441280

281+
public boolean isGreaterThan(final BigDecimal other) {
282+
return getAmount().compareTo(other) > 0;
283+
}
284+
442285
public boolean isGreaterThanZero() {
443286
return isGreaterThanZero(getMc());
444287
}
@@ -494,4 +337,31 @@ public Money zero(MathContext mc) {
494337
public MathContext getMc() {
495338
return mc != null ? mc : MoneyHelper.getMathContext();
496339
}
340+
341+
private BigDecimal defaultToZeroIfNull(final BigDecimal value) {
342+
BigDecimal result = BigDecimal.ZERO;
343+
if (value != null) {
344+
result = value;
345+
}
346+
return result;
347+
}
348+
349+
private BigDecimal defaultToNullIfZero(final BigDecimal value) {
350+
BigDecimal result = value;
351+
if (value != null && BigDecimal.ZERO.compareTo(value) == 0) {
352+
result = null;
353+
}
354+
return result;
355+
}
356+
357+
private Money checkCurrencyEqual(final Money money) {
358+
if (!isSameCurrency(money)) {
359+
throw new UnsupportedOperationException("currencies are different.");
360+
}
361+
return money;
362+
}
363+
364+
private boolean isSameCurrency(final Money money) {
365+
return getCurrencyCode().equals(money.getCurrencyCode());
366+
}
497367
}

‎fineract-core/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionData.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ public void setBalanceNumberOfDays(final Integer balanceNumberOfDays) {
403403

404404
public EndOfDayBalance toEndOfDayBalance(final Money openingBalance) {
405405
final MonetaryCurrency currency = openingBalance.getCurrency();
406-
Money endOfDayBalance = openingBalance.copy();
406+
Money endOfDayBalance = openingBalance;
407407
if (isDeposit() || isDividendPayoutAndNotReversed()) {
408408
endOfDayBalance = Money.of(currency, this.runningBalance);
409409
} else if (isWithdrawal() || isChargeTransactionAndNotReversed()) {
@@ -439,7 +439,7 @@ public EndOfDayBalance toEndOfDayBalanceBoundedBy(final Money openingBalance, fi
439439
final boolean isAllowOverdraft) {
440440

441441
final MonetaryCurrency currency = openingBalance.getCurrency();
442-
Money endOfDayBalance = openingBalance.copy();
442+
Money endOfDayBalance = openingBalance;
443443

444444
int numberOfDaysOfBalance = this.balanceNumberOfDays;
445445

‎fineract-core/src/main/java/org/apache/fineract/portfolio/savings/domain/interest/SavingsAccountTransactionDetailsForPostingPeriod.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public boolean occursOn(final LocalDate occursOnDate) {
6060

6161
public EndOfDayBalance toEndOfDayBalance(final Money openingBalance) {
6262
final MonetaryCurrency currency = openingBalance.getCurrency();
63-
Money endOfDayBalance = openingBalance.copy();
63+
Money endOfDayBalance = openingBalance;
6464
if (isDeposit() || isDividendPayoutAndNotReversed()) {
6565
endOfDayBalance = openingBalance.plus(getAmount(currency));
6666
} else if (isWithdrawal() || isChargeTransactionAndNotReversed()) {
@@ -94,7 +94,7 @@ public EndOfDayBalance toEndOfDayBalance(final LocalDateInterval periodInterval,
9494

9595
public EndOfDayBalance toEndOfDayBalanceBoundedBy(final Money openingBalance, final LocalDateInterval boundedBy) {
9696
final MonetaryCurrency currency = openingBalance.getCurrency();
97-
Money endOfDayBalance = openingBalance.copy();
97+
Money endOfDayBalance = openingBalance;
9898

9999
int numberOfDaysOfBalance = this.balanceNumberOfDays;
100100

0 commit comments

Comments
 (0)