Java multithreaded withdrawal and deposit banking

Question

i'm exercising Java multithreading on an example of simple banking funcions. The programm is able to perform multiple transactions at once on different threads.
All accounts transfer money to the following account, the last transferes money to the first.

I want some feedback so I can improve my coding standards and practices, specifically on multithreading.

Banking class:

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;

public class BankSystem
{

    public static void main(String[] args)
    {

        List<Account> accounts = new ArrayList<>();

        for (int i = 0; i < 5; i++)
        {
            accounts.add(new Account("user " + i, i, 100));
        }

        for (int i = 0; i < 5; i++)
        {
            if (i != 4)
            {
                accounts.get(i).setTransTarget(accounts.get(i + 1));
            } else
            {
                accounts.get(i).setTransTarget(accounts.get(0));
            }

            accounts.get(i).start();
        }

        try
        {
            for (int i = 0; i < 5; i++)
            {
                accounts.get(i).join();
            }
        } catch (InterruptedException e)
        {
            e.printStackTrace();
        }

        accounts.forEach(n -> System.out.println(n.getBalance()));
    }

    public static void transactMoney(Account sender, Account receiver, float amountToTransfer)
    {
        Account firstLock = sender;
        Account secondLock = receiver;

        if (sender.accountNumber < receiver.accountNumber)
        {
            firstLock = receiver;
            secondLock = sender;
        }

        System.out.println(Thread.currentThread().getName() + " waiting for first lock on user " + firstLock.accountNumber);
        synchronized (firstLock)
        {
            System.out.println(Thread.currentThread().getName() + " got first lock on user " + firstLock.accountNumber);
            System.out.println(Thread.currentThread().getName() + " waiting for second lock on user " + secondLock.accountNumber);
            synchronized (secondLock)
            {
                System.out.println(Thread.currentThread().getName() + " got second lock on user " + secondLock.accountNumber);
                sender.subtractBalance(amountToTransfer);

                int delayTime = ThreadLocalRandom.current().nextInt(100, 600);

                try
                {
                    Thread.sleep(delayTime);
                } catch (InterruptedException e)
                {
                    e.printStackTrace();
                }

                receiver.addBalance(amountToTransfer);
                System.out.println("\t" + Thread.currentThread().getName() + " withdraw $" + amountToTransfer);
            }

            System.out.println("\t" + Thread.currentThread().getName() + " released lock on user " + firstLock.accountNumber + " and " + secondLock.accountNumber);
        }
    }
}

Account class:

public class Account extends Thread
{
    volatile float balance;
    Account target;

    int accountNumber;

    public Account(String name, int accountNumber, int balance)
    {
        super(name);
        this.accountNumber = accountNumber;
        this.balance = balance;
    }

    @Override
    public void run()
    {

        for (int i = 0; i < 2; i++)
        {
            BankSystem.transactMoney(this, target, 100);
        }
    }

    public void setTransTarget(Account target)
    {
        this.target = target;
    }

    private void setBalance(float balance)
    {
        this.balance = balance;
    }

    public float getBalance()
    {
        return balance;
    }

    public void addBalance(float balance)
    {
        setBalance(getBalance() + balance);
    }

    public void subtractBalance(float balance)
    {
        setBalance(getBalance() - balance);
    }
}

Output of Console:

user 4 waiting for first lock on user 4
user 0 waiting for first lock on user 1
user 1 waiting for first lock on user 2
user 2 waiting for first lock on user 3
user 3 waiting for first lock on user 4
user 1 got first lock on user 2
user 2 got first lock on user 3
user 0 got first lock on user 1
user 4 got first lock on user 4
user 1 waiting for second lock on user 1
user 2 waiting for second lock on user 2
user 0 waiting for second lock on user 0
user 4 waiting for second lock on user 0
user 0 got second lock on user 0
    user 0 withdraw $100.0
user 4 got second lock on user 0
    user 0 released lock on user 1 and 0
user 0 waiting for first lock on user 1
user 1 got second lock on user 1
    user 4 withdraw $100.0
    user 4 released lock on user 4 and 0
user 4 waiting for first lock on user 4
user 3 got first lock on user 4
user 3 waiting for second lock on user 3
    user 1 withdraw $100.0
    user 1 released lock on user 2 and 1
user 0 got first lock on user 1
user 1 waiting for first lock on user 2
user 2 got second lock on user 2
user 0 waiting for second lock on user 0
user 0 got second lock on user 0
    user 0 withdraw $100.0
    user 0 released lock on user 1 and 0
    user 2 withdraw $100.0
    user 2 released lock on user 3 and 2
user 1 got first lock on user 2
user 2 waiting for first lock on user 3
user 3 got second lock on user 3
user 1 waiting for second lock on user 1
user 1 got second lock on user 1
    user 1 withdraw $100.0
    user 1 released lock on user 2 and 1
    user 3 withdraw $100.0
    user 3 released lock on user 4 and 3
user 3 waiting for first lock on user 4
user 2 got first lock on user 3
user 4 got first lock on user 4
user 2 waiting for second lock on user 2
user 4 waiting for second lock on user 0
user 2 got second lock on user 2
user 4 got second lock on user 0
    user 2 withdraw $100.0
    user 2 released lock on user 3 and 2
    user 4 withdraw $100.0
    user 4 released lock on user 4 and 0
user 3 got first lock on user 4
user 3 waiting for second lock on user 3
user 3 got second lock on user 3
    user 3 withdraw $100.0
    user 3 released lock on user 4 and 3
user 0 balance 100.0
user 1 balance 100.0
user 2 balance 100.0
user 3 balance 100.0
user 4 balance 100.0

Answer 1

Hello and welcome to code review! I hope you don't mind me being direct, but your class structure is simply a mess. We need to fix that before we go to the concurrency control.

Your Account is both an account and a single transaction from that account to some other account. This violates the single responsibility principle. It also makes it impossible to transfer money from one account to two other accounts simultaneously. You need to remove the transaction responsibility from the Account class and have the account class only contain information that is relevant to the state of that account only. Optimally the Account would only hold the account balance and the account number.

The account should not be a Thread. Accounts are not entities that act by themselves. They are just data containers. Instead you should have several instances of a TestClient class running concurrently as threads and a TestRunner that sets up and starts the clients.

TestRunner should initialize a single AccountController object and pass that reference to the TestClients during their construction. The AccountController provides the account transfer operation for transfering funds from one accout number to another.

public void transferFunds(String debtorIban, String creditorIban, int amount);

AccountController manages acquiring write locks to accounts for each transaction and the debiting and crediting of funds from accounts. In the simplest example you don't need a class to represent a transaction. The parameters to the transferFunds method provide all the needed information.