Skip to content

Commit

Permalink
Make sure all MonetaryAmounts are sanitized to avoid operation issues
Browse files Browse the repository at this point in the history
  • Loading branch information
sjanel committed Dec 19, 2021
1 parent 022fb33 commit 1bf7712
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 17 deletions.
31 changes: 24 additions & 7 deletions src/objects/include/monetaryamount.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <optional>
#include <string_view>

#include "cct_log.hpp"
#include "cct_string.hpp"
#include "currencycode.hpp"
#include "mathhelpers.hpp"
Expand All @@ -31,11 +32,15 @@ class MonetaryAmount {
constexpr MonetaryAmount() noexcept : _amount(0), _nbDecimals(0) {}

/// Constructs a MonetaryAmount representing the integer 'amount' with a neutral currency
constexpr explicit MonetaryAmount(std::integral auto amount) noexcept : _amount(amount), _nbDecimals(0) {}
constexpr explicit MonetaryAmount(std::integral auto amount) noexcept : _amount(amount), _nbDecimals(0) {
sanitizeIntegralPart();
}

/// Constructs a MonetaryAmount representing the integer 'amount' with a currency
constexpr explicit MonetaryAmount(std::integral auto amount, CurrencyCode currencyCode) noexcept
: _amount(amount), _nbDecimals(0), _currencyCode(currencyCode) {}
: _amount(amount), _nbDecimals(0), _currencyCode(currencyCode) {
sanitizeIntegralPart();
}

/// Construct a new MonetaryAmount from a double.
/// Precision is calculated automatically.
Expand All @@ -45,7 +50,8 @@ class MonetaryAmount {
/// number of decimals
constexpr MonetaryAmount(AmountType amount, CurrencyCode currencyCode, int8_t nbDecimals) noexcept
: _amount(amount), _nbDecimals(nbDecimals), _currencyCode(currencyCode) {
sanitize();
sanitizeDecimals();
sanitizeIntegralPart();
}

/// Constructs a new MonetaryAmount from a string, containing an optional CurrencyCode.
Expand Down Expand Up @@ -161,7 +167,7 @@ class MonetaryAmount {
constexpr MonetaryAmount toNeutral() const noexcept { return MonetaryAmount(_amount, CurrencyCode(), _nbDecimals); }

/// Truncate the MonetaryAmount such that it will contain at most maxNbDecimals
constexpr void truncate(int8_t maxNbDecimals) noexcept { sanitize(maxNbDecimals); }
constexpr void truncate(int8_t maxNbDecimals) noexcept { sanitizeDecimals(maxNbDecimals); }

/// Get a std::string_view on the currency of this amount
constexpr std::string_view currencyStr() const { return _currencyCode.str(); }
Expand All @@ -176,7 +182,9 @@ class MonetaryAmount {
private:
using UnsignedAmountType = uint64_t;

constexpr void simplify() noexcept {
static constexpr AmountType kMaxAmountFullNDigits = ipow(10, std::numeric_limits<AmountType>::digits10);

constexpr void simplifyDecimals() noexcept {
if (_amount == 0) {
_nbDecimals = 0;
} else {
Expand All @@ -186,13 +194,22 @@ class MonetaryAmount {
}
}

constexpr void sanitize(int8_t maxNbDecimals = std::numeric_limits<AmountType>::digits10 - 1) noexcept {
constexpr void sanitizeDecimals(int8_t maxNbDecimals = std::numeric_limits<AmountType>::digits10 - 1) noexcept {
const int8_t nbDecimalsToTruncate = _nbDecimals - maxNbDecimals;
if (nbDecimalsToTruncate > 0) {
_amount /= ipow(10, static_cast<uint8_t>(nbDecimalsToTruncate));
_nbDecimals -= nbDecimalsToTruncate;
}
simplify();
simplifyDecimals();
}

constexpr void sanitizeIntegralPart() {
if (_amount >= kMaxAmountFullNDigits) {
if (!std::is_constant_evaluated()) {
log::debug("Truncating last digit of integral part which is too big");
}
_amount /= 10;
}
}

constexpr bool isSane() const noexcept {
Expand Down
18 changes: 8 additions & 10 deletions src/objects/src/monetaryamount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include "cct_config.hpp"
#include "cct_exception.hpp"
#include "cct_log.hpp"
#include "stringhelpers.hpp"

namespace cct {
Expand Down Expand Up @@ -243,7 +242,6 @@ MonetaryAmount MonetaryAmount::operator+(MonetaryAmount o) const {
AmountType rhsAmount = o._amount;
int8_t resNbDecimals = SafeConvertSameDecimals(lhsAmount, rhsAmount, _nbDecimals, o._nbDecimals);
AmountType resAmount = lhsAmount + rhsAmount;
constexpr AmountType kMaxAmountFullNDigits = ipow(10, std::numeric_limits<AmountType>::digits10);
if (resAmount >= kMaxAmountFullNDigits || resAmount <= -kMaxAmountFullNDigits) {
resAmount /= 10;
--resNbDecimals;
Expand Down Expand Up @@ -325,12 +323,7 @@ MonetaryAmount MonetaryAmount::operator/(MonetaryAmount div) const {
AmountType lhsAmount = _amount;
AmountType rhsAmount = div._amount;
assert(rhsAmount != 0);
int8_t lhsNbDecimals = _nbDecimals;
int8_t rhsNbDecimals = div._nbDecimals;
int8_t lhsNbDigits = static_cast<int8_t>(ndigits(_amount));
const int negMult = ((lhsAmount < 0 && rhsAmount > 0) || (lhsAmount > 0 && rhsAmount < 0)) ? -1 : 1;
UnsignedAmountType lhs = std::abs(lhsAmount);
UnsignedAmountType rhs = std::abs(rhsAmount);
CurrencyCode resCurrency = CurrencyCode::kNeutral;
if (!_currencyCode.isNeutral() && !div.currencyCode().isNeutral()) {
if (CCT_UNLIKELY(_currencyCode != div.currencyCode())) {
Expand All @@ -345,13 +338,18 @@ MonetaryAmount MonetaryAmount::operator/(MonetaryAmount div) const {
// Indeed, on 64 bits the unsigned integral type can hold one more digit than its signed counterpart.
static_assert(std::numeric_limits<UnsignedAmountType>::digits10 > std::numeric_limits<AmountType>::digits10);

int8_t lhsNbDigits = static_cast<int8_t>(ndigits(_amount));
const int8_t lhsNbDigitsToAdd = std::numeric_limits<UnsignedAmountType>::digits10 - lhsNbDigits;
lhs *= ipow(static_cast<UnsignedAmountType>(10), static_cast<uint8_t>(lhsNbDigitsToAdd));
lhsNbDecimals += lhsNbDigitsToAdd;
UnsignedAmountType lhs = static_cast<UnsignedAmountType>(std::abs(lhsAmount)) *
ipow(static_cast<UnsignedAmountType>(10), static_cast<uint8_t>(lhsNbDigitsToAdd));
UnsignedAmountType rhs = static_cast<UnsignedAmountType>(std::abs(rhsAmount));

int8_t lhsNbDecimals = _nbDecimals + lhsNbDigitsToAdd;

lhsNbDigits += lhsNbDigitsToAdd;

UnsignedAmountType totalIntPart = 0;
int8_t nbDecimals = lhsNbDecimals - rhsNbDecimals;
int8_t nbDecimals = lhsNbDecimals - div._nbDecimals;
int8_t totalPartNbDigits;
do {
totalIntPart += lhs / rhs; // Add integral part
Expand Down
2 changes: 2 additions & 0 deletions src/objects/test/monetaryamount_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ TEST(MonetaryAmountTest, Divide) {
EXPECT_EQ(MonetaryAmount("487.76 EUR") / MonetaryAmount("1300.5 EUR"), MonetaryAmount("0.3750557477893118"));
EXPECT_THROW(MonetaryAmount("100") / MonetaryAmount("0.00000000000000001"), exception);
EXPECT_EQ(MonetaryAmount("10") / MonetaryAmount("0.0000000000000001"), MonetaryAmount("100000000000000000"));
EXPECT_EQ(MonetaryAmount("1000000000 KRW") / MonetaryAmount("922337203685477580 KRW"),
MonetaryAmount("0.00000000108420217"));
}

TEST(MonetaryAmountTest, Multiply) {
Expand Down

0 comments on commit 1bf7712

Please sign in to comment.