Skip to content

Commit

Permalink
Avoid losing precision when the user enters an exchange rate.
Browse files Browse the repository at this point in the history
We were storing the rate as originAmount/convertedAmount, which was
limited to the precision of the currencies. Now we store directly the
entered rate.

Fixes #479
  • Loading branch information
rivaldi8 committed Apr 18, 2016
1 parent e7ee18e commit 66cc533
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 18 deletions.
33 changes: 33 additions & 0 deletions app/src/main/java/org/gnucash/android/model/Price.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

import org.gnucash.android.util.TimestampHelper;


import java.math.BigDecimal;
import java.sql.Timestamp;
import java.text.DecimalFormat;
import java.text.NumberFormat;

/**
* Model for commodity prices
Expand Down Expand Up @@ -37,6 +41,19 @@ public Price(String commodityUID, String currencyUID){
mDate = TimestampHelper.getTimestampFromNow();
}

/**
* Create new instance with the GUIDs of the commodities and the specified exchange rate.
* @param commodity1UID GUID of the origin commodity
* @param commodity2UID GUID of the target commodity
* @param exchangeRate exchange rate between the commodities
*/
public Price(String commodity1UID, String commodity2UID, BigDecimal exchangeRate) {
this(commodity1UID, commodity2UID);
// Store 0.1234 as 1234/10000
setValueNum(exchangeRate.unscaledValue().longValue());
setValueDenom(BigDecimal.ONE.scaleByPowerOfTen(exchangeRate.scale()).longValue());
}

public String getCommodityUID() {
return mCommodityUID;
}
Expand Down Expand Up @@ -123,4 +140,20 @@ public void reduce() {
mValueDenom /= commonDivisor;
}
}

/**
* Returns the exchange rate as a string formatted with the default locale.
*
* <p>It will have up to 6 decimal places.
*
* <p>Example: "0.123456"
*/
@Override
public String toString() {
BigDecimal numerator = new BigDecimal(mValueNum);
BigDecimal denominator = new BigDecimal(mValueDenom);
DecimalFormat formatter = (DecimalFormat) NumberFormat.getNumberInstance();
formatter.setMaximumFractionDigits(6);
return formatter.format(numerator.divide(denominator));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,21 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa
Commodity currencyCommodity = commoditiesDbAdapter.getCommodity(mTargetCurrency.getCurrencyCode());
String currencyUID = currencyCommodity.getUID();
PricesDbAdapter pricesDbAdapter = PricesDbAdapter.getInstance();
Pair<Long, Long> price = pricesDbAdapter.getPrice(commodityUID, currencyUID);
Pair<Long, Long> pricePair = pricesDbAdapter.getPrice(commodityUID, currencyUID);

if (price.first > 0 && price.second > 0) {
if (pricePair.first > 0 && pricePair.second > 0) {
// a valid price exists
BigDecimal numerator = new BigDecimal(price.first);
BigDecimal denominator = new BigDecimal(price.second);
DecimalFormat formatter = (DecimalFormat) NumberFormat.getNumberInstance();
mExchangeRateInput.setText(formatter.format(numerator.divide(denominator, MathContext.DECIMAL32)));
Price price = new Price(commodityUID, currencyUID);
price.setValueNum(pricePair.first);
price.setValueDenom(pricePair.second);
mExchangeRateInput.setText(price.toString());

BigDecimal numerator = new BigDecimal(pricePair.first);
BigDecimal denominator = new BigDecimal(pricePair.second);
// convertedAmount = mOriginAmount * numerator / denominator
BigDecimal convertedAmount = mOriginAmount.asBigDecimal().multiply(numerator)
.divide(denominator, currencyCommodity.getSmallestFractionDigits(), BigDecimal.ROUND_HALF_EVEN);
DecimalFormat formatter = (DecimalFormat) NumberFormat.getNumberInstance();
mConvertedAmountInput.setText(formatter.format(convertedAmount));
}

Expand Down Expand Up @@ -194,17 +198,24 @@ public Dialog onCreateDialog(Bundle savedInstanceState) {
* Converts the currency amount with the given exchange rate and saves the price to the db
*/
private void transferFunds() {
Price price = null;

CommoditiesDbAdapter commoditiesDbAdapter = CommoditiesDbAdapter.getInstance();
String originCommodityUID = commoditiesDbAdapter.getCommodityUID(mOriginAmount.getCurrency().getCurrencyCode());
String targetCommodityUID = commoditiesDbAdapter.getCommodityUID(mTargetCurrency.getCurrencyCode());

if (mExchangeRateRadioButton.isChecked()){
BigDecimal rate;
String exchangeRateString = mExchangeRateInput.getText().toString();
DecimalFormat formatter = (DecimalFormat) NumberFormat.getNumberInstance();
formatter.setParseBigDecimal(true);
BigDecimal rate;
try {
rate = (BigDecimal) formatter.parse(exchangeRateString);
} catch (ParseException e) {
mExchangeRateInputLayout.setError(getString(R.string.error_invalid_exchange_rate));
return;
}
price = new Price(originCommodityUID, targetCommodityUID, rate);
mConvertedAmount = mOriginAmount.multiply(rate);
}

Expand All @@ -217,22 +228,20 @@ private void transferFunds() {

BigDecimal amount = TransactionFormFragment.parseInputToDecimal(convertedAmount);
mConvertedAmount = new Money(amount, Commodity.getInstance(mTargetCurrency.getCurrencyCode()));
}

if (mOnTransferFundsListener != null) {
PricesDbAdapter pricesDbAdapter = PricesDbAdapter.getInstance();
CommoditiesDbAdapter commoditiesDbAdapter = CommoditiesDbAdapter.getInstance();
Price price = new Price(commoditiesDbAdapter.getCommodityUID(mOriginAmount.getCurrency().getCurrencyCode()),
commoditiesDbAdapter.getCommodityUID(mTargetCurrency.getCurrencyCode()));
price.setSource(Price.SOURCE_USER);
// fractions cannot be exacted represented by BigDecimal.
price = new Price(originCommodityUID, targetCommodityUID);
// fractions cannot be exactly represented by BigDecimal.
price.setValueNum(mConvertedAmount.getNumerator() * mOriginAmount.getDenominator());
price.setValueDenom(mOriginAmount.getNumerator() * mConvertedAmount.getDenominator());
price.reduce();
pricesDbAdapter.addRecord(price);
}

price.setSource(Price.SOURCE_USER);
price.reduce();
PricesDbAdapter.getInstance().addRecord(price);

if (mOnTransferFundsListener != null)
mOnTransferFundsListener.transferComplete(mConvertedAmount);
}

dismiss();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright (c) 2016 Àlex Magaz Graça <rivaldi8@gmail.com>
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.gnucash.android.test.unit.model;

import org.gnucash.android.model.Price;
import org.junit.Test;

import java.math.BigDecimal;
import java.util.Locale;

import static org.assertj.core.api.Assertions.assertThat;


public class PriceTest {
@Test
public void creatingFromExchangeRate_ShouldGetPrecisionRight() {
Locale.setDefault(Locale.US);

String exchangeRateString = "0.123456";
BigDecimal exchangeRate = new BigDecimal(exchangeRateString);
Price price = new Price("commodity1UID", "commodity2UID", exchangeRate);
assertThat(price.toString()).isEqualTo(exchangeRateString);

// ensure we don't get more decimal places than needed (0.123000)
exchangeRateString = "0.123";
exchangeRate = new BigDecimal(exchangeRateString);
price = new Price("commodity1UID", "commodity2UID", exchangeRate);
assertThat(price.toString()).isEqualTo(exchangeRateString);
}

@Test
public void toString_shouldUseDefaultLocale() {
Locale.setDefault(Locale.GERMANY);

String exchangeRateString = "1.234";
BigDecimal exchangeRate = new BigDecimal(exchangeRateString);
Price price = new Price("commodity1UID", "commodity2UID", exchangeRate);
assertThat(price.toString()).isEqualTo("1,234");
}
}

0 comments on commit 66cc533

Please sign in to comment.