<p><b>@chimp1984</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3586#discussion_r347070743">core/src/main/java/bisq/core/payment/PaymentAccountManager.java</a>:</p>
<pre style='color:#555'>> +
+            }
+            accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload());
+        }
+        return paymentAccount;
+    }
+
+    public void removePaymentAccount(String id) {
+        PaymentAccount paymentAccount = user.getPaymentAccount(id);
+        if (paymentAccount == null) {
+            throw new NotFoundException(format("Payment account %s not found", id));
+        }
+        boolean isPaymentAccountUsed = openOfferManager.getObservableList().stream()
+                .anyMatch(openOffer -> id.equals(openOffer.getOffer().getMakerPaymentAccountId()));
+        if (isPaymentAccountUsed) {
+            throw new PaymentAccountInUseException(format("Payment account %s is used for open offer", id));
</pre>
<p>I disagree. I think the method removePaymentAccount should do what its name suggests and not throw an unchecked exception if the valid case happens that the user tries to remove an account but has an offer open using it.<br>
Bisq is using exceptions for exceptional cases not for flow control. I think we should stick with that patter and not introduce tons of exceptions for all kind of checks.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3586#discussion_r347070801">desktop/src/main/java/bisq/desktop/main/account/content/altcoinaccounts/AltCoinAccountsDataModel.java</a>:</p>
<pre style='color:#555'>>      }
 
     public boolean onDeleteAccount(PaymentAccount paymentAccount) {
-        boolean isPaymentAccountUsed = openOfferManager.getObservableList().stream()
-                .filter(o -> o.getOffer().getMakerPaymentAccountId().equals(paymentAccount.getId()))
-                .findAny()
-                .isPresent();
-        isPaymentAccountUsed = isPaymentAccountUsed || tradeManager.getTradableList().stream()
-                .filter(t -> t.getOffer().getMakerPaymentAccountId().equals(paymentAccount.getId()) ||
-                        paymentAccount.getId().equals(t.getTakerPaymentAccountId()))
-                .findAny()
-                .isPresent();
-        if (!isPaymentAccountUsed)
-            user.removePaymentAccount(paymentAccount);
-        return isPaymentAccountUsed;
+        try {
+            paymentAccountManager.removePaymentAccount(paymentAccount.getId());
</pre>
<p>I prefer to have then 2 methods if you need that or just do the lookup in your client code.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/3586?email_source=notifications&email_token=AJFFTNVMCYKGAPGCANCNQ33QT5JKTA5CNFSM4JLIFYS2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLZXU2I#discussion_r347070743">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNXDVGAJ2RUXFHMVTGDQT5JKTANCNFSM4JLIFYSQ">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNXZT5ORBI4XJREH4K3QT5JKTA5CNFSM4JLIFYS2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLZXU2I.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/3586?email_source=notifications\u0026email_token=AJFFTNVMCYKGAPGCANCNQ33QT5JKTA5CNFSM4JLIFYS2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLZXU2I#discussion_r347070743",
"url": "https://github.com/bisq-network/bisq/pull/3586?email_source=notifications\u0026email_token=AJFFTNVMCYKGAPGCANCNQ33QT5JKTA5CNFSM4JLIFYS2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLZXU2I#discussion_r347070743",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>