Skip to content

Commit

Permalink
Unit tests correction after fixing change in Value division: method I…
Browse files Browse the repository at this point in the history
…ntegerValueStorage.DivideValueStorage had reversed dividend and divisor for the case INTEGER/AMOUNT. Though the original Ledger code still contains the mistake, it was decised to correct c# code and update related Boost unit tests (that relies on wrong division results as well). The main motivation for the fix was to minimize negative impact on integration capabilities. Note: the issue is not revealed by Ledger test set.
  • Loading branch information
Dmitry Merzlyakov committed Sep 23, 2021
1 parent dc803b8 commit 2d1570c
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
8 changes: 7 additions & 1 deletion Source/NLedger.IntegrationTests/unit/TestValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,13 @@ public void AutoTestCase_Value_TestDivision()
v5 /= new Value(2L);
Assert.Equal(v5, new Value(4L));
v5 /= new Value(new Amount("8"));
Assert.Equal(v5, new Value(new Amount("2")));

// [DM] The original Ledger unit test is corrected here to reflect the fix in INTEGER/AMOUNT division
// (see comments in ValueStorage.cs; IntegerValueStorage; DivideValueStorage
// Even from logical standpoint, 4/8=0.5, not 2
Assert.Equal(v5, new Value(new Amount("0.5")));
v5 = new Value(2L); // Set to 2 to pass next test steps
// Original unit test row - Assert.Equal(v5, new Value(new Amount("2")));

v16 /= new Value(2L);
v16 /= new Value(new Amount("2"));
Expand Down
7 changes: 7 additions & 0 deletions Source/NLedger/Values/ValueStorage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,13 @@ protected override IValueStorage DivideValueStorage(IValueStorage valueStorage)
if (valueStorage.SafeType() == ValueTypeEnum.Integer)
return new IntegerValueStorage(AsLong / valueStorage.AsLong);

// [DM] Initial c# code for [Integer]/[Amount] division: return new AmountValueStorage(valueStorage.AsAmount / AsAmount);
// This initial code reflected the original Ledger code that is likely contains a mistake:
// value.cc - value_t& value_t::operator/=(const value_t& val); case INTEGER/case AMOUNT:
// set_amount(val.as_amount() / as_long());
// In the source code the dividend and divisor are reversed that causes wrong division results. Notice that previous division INTEGER/INTEGER is correct.
// It was decided to fix this problem in c# code since wrong division results cause negative effect on integrated capabilities.

if (valueStorage.SafeType() == ValueTypeEnum.Amount)
return new AmountValueStorage(AsAmount / valueStorage.AsAmount);

Expand Down

0 comments on commit 2d1570c

Please sign in to comment.