How to make multiple operation in ifPresent with Optional on Java?

Question

What I do is simply user can do trade but first I need to check balance of accounts(usdt & bitcoin account). Simple thing is that for buying I only check usdt account balance and for selling I only check crypto balance but the problem is that these accounts can be null too, thats why I need to use Optional.

Here is my comparing code;

private boolean isBalanceAvailableForTrade(TradeRequestDto requestDto, List<Account> accountList) {
        Optional<Account> dollarAccount = accountList.stream()
                .filter(account -> account.getAccountCurrencyType().equals(Currency.USDT.toString()))
                .findFirst();
        Optional<Account> cryptoAccount = accountList.stream()
                .filter(account -> account.getAccountCurrencyType().equals(requestDto.getCurreny()))
                .findFirst();

        if (requestDto.getOperationType().equalsIgnoreCase(BUY)) {
            if (dollarAccount.get().getAmount().compareTo(BigDecimal.valueOf(0)) > 0) {
                return true;
            }
            //TODO throw no available amount exception etc.
            return false;
        }
        if (requestDto.getOperationType().equalsIgnoreCase(SELL)) {
            if(cryptoAccount.get().getAmount().compareTo(BigDecimal.valueOf(0)) > 0) {
                return true;
            }
            //TODO throw no available amount exception etc.
            return false;
        }
        return false;
    }

My problem is actually I am not checking if the accounts are null above, the line

`if (dollarAccount.get().getAmount().compareTo(BigDecimal.valueOf(0)) > 0)` may throw *NullPointerException*.

I need something like :

dollarAccount.ifPresent(()-> {then do multiple line of code/operation} ).orElseThrow(some exception)

I am pretty near to solution I think but I cannot figure out how to implement Optional for multiple check(balance check and null check). How should I refactor this piece of code properly ?

Side note: I am not sure if the following account check is also best practice or not and if you can give suggestion, it is more than welcome

Optional<Account> dollarAccount = accountList.stream()
                    .filter(account -> account.getAccountCurrencyType().equals(Currency.USDT.toString()))
                    .findFirst();

Answer 1

I suggest you to extract the logic of finding an account and getting its balance into a separate method. It'll make the code more cohesive and concise.

Method getAmount() returns a BigDecimal and there's no need to deal with the Optional inside isBalanceAvailableForTrade() at all.

Hence you are OK with throwing the exception in the case if an account for the given currency isn't found it's better to make use of the orElseThrow() inside the getAmount() method, rather than bother will optional object downstream in the call chain.

problem is actually I am not checking if the accounts are null

To avoid NPE it's enough to add one more filter to a stream (take a look at getAmount() method), and it has to be the first operation in the pipeline.

private boolean isBalanceAvailableForTrade(TradeRequestDto requestDto, List<Account> accountList) {
        BigDecimal dollarAccAmount = getAmount(accountList, Currency.USDT.toString());
        BigDecimal cryptoAccAmount = getAmount(accountList, requestDto.getCurreny());

        if (requestDto.getOperationType().equalsIgnoreCase(BUY)) {
            if (dollarAccAmount.compareTo(BigDecimal.ZERO) > 0) {
                return true;
            }
            throw new NoAvailableAmountException();
        }
        if (requestDto.getOperationType().equalsIgnoreCase(SELL)) {
            if (cryptoAccAmount.compareTo(BigDecimal.ZERO) > 0) {
                return true;
            }
            throw new NoAvailableAmountException();
        }
        return false;
    }

    private BigDecimal getAmount(List<Account> accountList, String currency) {
        return accountList.stream()
                .filter(account -> account != null)
                .filter(account -> account.getAccountCurrencyType().equals(currency))
                .findFirst()
                .map(Account::getAmount)
                .orElseThrow(() -> new RuntimeException("unable to find an Account for currency " + currency));
    }

I need something like :

dollarAccount.ifPresent(()-> {then do multiple line of code/operation} ).orElseThrow(some exception)

For this purpose Optional class has been provided with methods filter(), map(), flatMap(). And since Java 9 we have stream() which create either an empty stream or a stream with a single element. That allows you to chain mapping and filtering operations on the optional object as much as you need or turn optional into a stream.

Please, note that operation map(accountList::getAmount) inside the method getAmount() is invoked not on the element of the stream, but on the Optional object, returned by the terminal operation findFirst().

https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Optional.html#method.summary

Answer 2

You should avoid using get on an Optional. You can unpack an optional using orElseThrow as follows:

Account account = dollarAccount.orElseThrow(() -> new NotFoundException("Dollar account not found!));

This will either return an Account or throw an exception if the optinal is emtpy.

For your side note:

Your stream looks correct, it will return an empty optinal if accountList is empty or if the filter would not find any account. Otherwise, the filtering will stop at the first account found.

Answer 3

.orElseThrow() returns the contained value, so you just use it instead of .get():

dollarAccount.orElseThrow(() -> new Exception()).getAmount().compareTo(BigDecimal.valueOf(0)) > 0

Maybe just split it up, so that it doesn't get too long (and possibly drop the if):

Account account = dollarAccount.orElseThrow(() -> new Exception());
return account.getAmount().compareTo(BigDecimal.valueOf(0)) > 0;