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?
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);
}