javamultithreadingconcurrencythread-safetyjava-threads

Thread Safety Multiple Approaches


I'm developing an application which is meant to work in multi threading environment. The code sample looks like -

private Map<String, Account> accountMap;    // Account is a POJO
     
public Double processTransaction(Transaction transaction) {     // Transaction is a POJO
        Account account = accountMap.get(transaction.getAccountNumber());

        synchronized (account) {
            TransactionType mode = transaction.getTransactionType();
            switch (mode) {
                case DEPOSIT:
                    Double balance = account.getBalance();
                    balance += transaction.getBalance();
                    account.setBalance(balance);
                    accountMap.put(transaction.getAccountNumber(), account);
                    break;

                case WITHDRAW:
                    Double presentBalance = account.getBalance();
                    if(presentBalance < transaction.getBalance()) {
                        // throw some exception
                    }
                    presentBalance = presentBalance - transaction.getBalance();
                    account.setBalance(presentBalance);
                    accountMap.put(transaction.getAccountNumber(), account);
                    break;

            }
        }
        return accountMap.get(transaction.getAccountNumber()).getBalance();
    }

The other possible approach could be defining accountMap as a concurrent HashMap instead of using synchronized block.

My question is, are the above approaches correct and what else approaches could be used to ensure thread safety?


Solution

  • The calls to put() are redundant because the mapping is already present. It looks like the only thing this method really does is adjust the balance of an individual account. I would have Account provide an adjustment method that synchronizes internally:

    public synchronized double adjustBalance(double delta) {
        return this.balance += delta;
    }
    

    Then call it with a simple if to negate the amount:

    public Double processTransaction(Transaction transaction) {
        double delta = transaction.getBalance();
        if (transaction.getTransactionType() == TransactionType.WITHDRAW) {
            delta = -delta;
        }
        return accountMap.get(transaction.getAccountNumber()).adjustBalance(delta);
    }