AlkValue has no constructor for qint64 aka signed64. That saves us compiler warning and maybe test will work again on FreeBSD.
Details
Details
- Reviewers
tbaumgart - Group Reviewers
KMyMoney - Commits
- R261:724691bfdc3b: Use unsigned int for denominator in MyMoneyMoney
All tests pass.
Diff Detail
Diff Detail
- Repository
- R261 KMyMoney
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| kmymoney/mymoney/mymoneymoney.cpp | ||
|---|---|---|
| 155 | This introduces an implicit conversion for Amount and denom from int to QString before they are passed on to arg() . What is the benefit here? Please explain. | |
| kmymoney/mymoney/mymoneymoney.cpp | ||
|---|---|---|
| 155 | Yes, but it does not apply in our case. It is just wrong! I added the following testcase which shows the problem. The original version works as expected, yours fails. BTW. the second warning on diff --git a/kmymoney/mymoney/tests/mymoneymoney-test.cpp b/kmymoney/mymoney/tes index ae07f075..b9645f31 100644 --- a/kmymoney/mymoney/tests/mymoneymoney-test.cpp +++ b/kmymoney/mymoney/tests/mymoneymoney-test.cpp @@ -38,7 +38,7 @@ void MyMoneyMoneyTest::init() m_2 = new MyMoneyMoney(2, 100); m_3 = new MyMoneyMoney(123, 1); m_4 = new MyMoneyMoney(1234, 1000); - m_5 = new MyMoneyMoney(195883, 100000); + m_5 = new MyMoneyMoney(static_cast<qint64>(195883), 100000); m_6 = new MyMoneyMoney(1.247658435, 1000000000); MyMoneyMoney::setDecimalSeparator('.'); @@ -75,9 +75,16 @@ void MyMoneyMoneyTest::testIntConstructor() QVERIFY(m_0->valueRef().get_num() == 3); QVERIFY(m_0->valueRef().get_den() == 25); + QVERIFY(m_5->valueRef().get_num() == 195883); + QVERIFY(m_5->valueRef().get_den() == 100000); + + QVERIFY(m_6->valueRef().get_num() == 249531687); + QVERIFY(m_6->valueRef().get_den() == 200000000); + MyMoneyMoney a(123, 10000); QVERIFY(a.valueRef().get_num() == 123); QVERIFY(a.valueRef().get_den() == 10000); + } void MyMoneyMoneyTest::testAssignment() I think, I will add this test to master. | |
Comment Actions
I misused multi-arg overload but fixed it right now.
| kmymoney/mymoney/mymoneymoney.cpp | ||
|---|---|---|
| 155 | You're right. Please do add this test, as it will enhance our code quality. | |