[bisq-network/bisq-core] Improve text (#158)

sqrrm notifications at github.com
Sat Aug 18 21:14:49 UTC 2018


sqrrm requested changes on this pull request.

Please fix getAdjustedAmount function, either with explanation on what it does if current behavior is expected or rounding of return data for all cases to match expectation.

>          amount = price.getAmountByVolume(volume);
 
         // We want only 4 decimal places
         long rounded = Math.round((double) amount.value / 10000d) * 10000;
         if (rounded > maxTradeLimit) {
             // If we are above out trade limit we reduce the amount by the correlating 10 EUR volume
-            rounded = Math.min(maxTradeLimit, rounded - amountByVolumeRoundedToFactor.value);
+            long reduced = rounded - amountByVolumeRoundedToFactor.value;
+            if (reduced > Restrictions.getMinTradeAmount().value)
+                rounded = Math.min(maxTradeLimit, reduced);

If the amount is more than `factor` more than `maxTradeLimit` the returned value will be `maxTradeLimit` which is not rounded. This is later rounded down to factor when this function is called again from MutableOfferDataModel:702 so everything works in when running the code.

This is somewhat confusing while reading the code though, I expect `getAdjustedAmount` to return a rounded amount for every input value.

> @@ -167,20 +168,28 @@ public static Coin getAdjustedAmountForHalCash(Coin amount, Price price, long ma
     private static Coin getAdjustedAmount(Coin amount, Price price, long maxTradeLimit, String currencyCode, int factor) {
         // Amount must result in a volume of min factor units of the fiat currency, e.g. 1 EUR or 10 EUR in case of Halcash
         Volume volumeRoundedToFactor = Volume.parse(String.valueOf(factor), currencyCode);
+        if (volumeRoundedToFactor.getValue() <= 0)
+            return Coin.ZERO;
+
         Coin amountByVolumeRoundedToFactor = price.getAmountByVolume(volumeRoundedToFactor);
         // We set min amount so it has a volume of 10 EUR
         if (amount.compareTo(amountByVolumeRoundedToFactor) < 0)
             amount = amountByVolumeRoundedToFactor;
 
         // We adjust the amount so that the volume is a multiple of 10 EUR

Refers to a multiple of `factor`

> @@ -167,20 +168,28 @@ public static Coin getAdjustedAmountForHalCash(Coin amount, Price price, long ma
     private static Coin getAdjustedAmount(Coin amount, Price price, long maxTradeLimit, String currencyCode, int factor) {
         // Amount must result in a volume of min factor units of the fiat currency, e.g. 1 EUR or 10 EUR in case of Halcash
         Volume volumeRoundedToFactor = Volume.parse(String.valueOf(factor), currencyCode);
+        if (volumeRoundedToFactor.getValue() <= 0)
+            return Coin.ZERO;
+
         Coin amountByVolumeRoundedToFactor = price.getAmountByVolume(volumeRoundedToFactor);
         // We set min amount so it has a volume of 10 EUR

These comments refer to a multiple of `factor`

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq-core/pull/158#pullrequestreview-147440468
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20180818/2699a53e/attachment-0001.html>


More information about the bisq-github mailing list