[bisq-network/bisq] Build out 'createoffer' API method (#4559)

sqrrm notifications at github.com
Fri Oct 2 11:06:30 UTC 2020


@sqrrm commented on this pull request.

> The `createoffer` and `createpaymentacct` methods have complex argument lists, and different devs may want them defined in in different ways. We should discuss?
> 
> Right now, `createoffer` also places the offer, and problems might be created by placing it before giving users a chance to review details first. I thought the default behavior should be to cache the new offer (with ID) in the daemon, show it to the user, and let them place the new offer with a different `confirmoffer offer-id` command. But I need some consensus on that too.

For an API I'm not sure there is a use case for the `confirm offer` extra step. Automated code wouldn't gain anything and UIs should probably implement that locally.

> +import java.util.Map;
+import java.util.function.Predicate;
+
+import static java.util.Arrays.stream;
+
+class NegativeNumberOptions {
+
+    private final Map<Integer, String> negativeNumberParams = new HashMap<>();
+
+    String[] removeNegativeNumberOptions(String[] args) {
+        // Cache any negative number params that will be rejected by the parser.
+        // This should be called before command line parsing.
+        for (int i = 0; i < args.length; i++) {
+            if (isNegativeNumber.test(args[i])) {
+                String param = args[i];
+                negativeNumberParams.put(i - 1, new BigDecimal(param).toString());

Why put at `i-1`? Wouldn't it be more natural to use `i`?

> +                nonOptionArgsClone.remove(idx);
+                nonOptionArgsClone.add(idx, v);

How about `nonOptionArgsClone.set(idx, v);`?

> +                    if (nonOptionArgs.size() < 9)
+                        throw new IllegalArgumentException("incorrect parameter count,"
+                                + " expecting buy | sell, payment acct id, currency code, amount, min amount,"
+                                + " use-market-based-price, fixed-price | mkt-price-margin, security-deposit");

Expected more than 8 options but only 8 expected options listed

-- 
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/pull/4559#pullrequestreview-500317915
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20201002/9e309fef/attachment.html>


More information about the bisq-github mailing list