Object-oriented Bank class

Question

I am quite new to programming and was given an assignment in a job recruitment(now finished) to program a simple Bank class and other relevant classes based on this interface:

Account CreateAccount(Person customer, Money initialDeposit)
Account[] GetAccountsForCustomer(Person customer)
void Deposit(Account to, Money amount)
void Withdraw(Account from, Money amount)
void Transfer(Account from, Account to, Money amount)

Along with this interface I was given several user scenarios:

CreateAccount:

1. Create an account for a given person with a given initial deposit. The name of the account is customer's name followed by a serial number that starts at 1. This serial number is per customer.
2. The next accounts should have the same name, but a higher serial number.
3. The account should not be created if the person can not cover the initial deposit.

GetAccountForCustomer:

1. Get all the accounts for a given customer.
2. Give an empty array if the person doesn't have an account in the bank.

Deposit:

1. The person who owns the account deposits money.
2. The money can not be deposited if the person can not afford it.

Withdraw:

1. The person who owns the account withdraws money.
2. The money can not be withdrawn if the account has insufficient funds.

Transfer:

1. Transfer money from one account to another
2. The money should not be transferred if the account has insufficient funds.

Please create unit tests to show that the code works as intended.

I had to make several design choices along the way, and as an inexperienced programmer I don't know enough about proper code style and software architecture. The main code is less then 300 lines.

I am especially uncertain about my implementation of:

• Exception handling, return true/false or null, or throw exception? Where in the call stack should I throw the exception?
• Unit testing: My unit testing code seems quite bloated, is this an inherent attribute of unit testing or is there a better way?
• Decomposing methods too much?
• Anything else I have forgotten to consider?

Code: https://github.com/odgaard/DIPS-Challenge/

Here is the main Bank.cs class:

using System;
namespace DIPS_Challenge
{
    public class Bank : IBankable
    {
        private string _bankName;

        public Bank(string name)
        {
            BankName = name;
        }

        public Account CreateAccount(Person customer, Money initialDeposit)
        {
            if (_validPersonWithdrawTransaction(customer, initialDeposit))
            {
                var newAccount = new Account(initialDeposit, customer);
                customer.Money = new Money(customer.Money.Value - initialDeposit.Value);
                customer.AddAccounts(newAccount);
                return newAccount;
            }
            return null;
        }

        public Account[] GetAccountsForCustomer(Person customer) => customer.Accounts;

        // This method only supports one type of currency.
        private bool _requestPersonHasSufficientFunds(Person owner, Money amount) => (owner.Money.Value >= amount.Value);

        // This method only supports one type of currency.
        private bool _requestAccountHasSufficientFunds(Account transfer, Money amount) => (transfer.Money.Value >= amount.Value);

        private bool _requestMoneyIsPositive(Money amount) => (amount.Value > 0);

        private bool _validPersonWithdrawTransaction(Person owner, Money amount)
        {
            if (!_requestMoneyIsPositive(amount))
            {
                throw new ArgumentException("Invalid value, Negative " + amount.Value);
            }

            if (!_requestPersonHasSufficientFunds(owner, amount))
            {
                throw new ArgumentException("Person has insufficient funds: " + owner.Money.Value + " < " + amount.Value);
            }

            return true;
        }

        private bool _validAccountWithdrawTransaction(Account transfer, Money amount)
        {

            if (!_requestMoneyIsPositive(amount))
            {
                throw new ArgumentException("Invalid value, Negative " + amount.Value);
            }

            if (!_requestAccountHasSufficientFunds(transfer, amount))
            {
                throw new ArgumentException("Account has insufficient funds: " + transfer.Money.Value + " < " + amount.Value);
            }

            return true;
        }

        private bool _validAccountDepositTransaction(Account transfer, Money amount)
        {
            if (!_requestMoneyIsPositive(amount))
            {
                throw new ArgumentException("Invalid value, Negative " + amount.Value);
            }

            return true;
        }

        private bool _validAccountTransferTransaction(Account from, Account to, Money amount) => (
               _validAccountDepositTransaction(to, amount)
            && _validAccountWithdrawTransaction(from, amount)
            );

        // This method only supports one type of currency.
        private void _performAccountDepositTransaction(Account transfer, Money amount) => 
            transfer.Money = new Money(transfer.Money.Value + amount.Value);

        // This method only supports one type of currency.
        private void _performAccountWithdrawTransaction(Account transfer, Money amount) =>
            transfer.Money = new Money(transfer.Money.Value - amount.Value);

        public void Deposit(Account to, Money amount)
        {
            if (_validAccountDepositTransaction(to, amount))
            {
                _performAccountDepositTransaction(to, amount);
            }       
        }

        public void Withdraw(Account from, Money amount)
        {
            if(_validAccountWithdrawTransaction(from, amount))
            {
                _performAccountWithdrawTransaction(from, amount);
            }
        }

        public void Transfer(Account from, Account to, Money amount)
        {
            if(_validAccountTransferTransaction(from, to, amount))
            {
                Withdraw(from, amount);
                Deposit(to, amount);
            }
        }

        public string BankName { get => _bankName; set => _bankName = value; }
    }
}

Here is the unit testing:

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace DIPS_Challenge
{

    [TestClass]
    public class PersonTest
    {
        [TestMethod]
        public void PersonCreate()
        {
            // Setup test variables
            var testUser = new Person("Test User");

            // Perform test
            Assert.AreEqual(testUser.Name, "Test User");
        }    
    }

    [TestClass]
    public class MoneyTest
    {
        [TestMethod]
        public void MoneyCreate()
        {
            // Setup test variables
            decimal testAmount = 1000;

            // Setup test objects
            var testMoney = new Money(testAmount);

            // Perform test
            Assert.AreEqual(testMoney.Value, testAmount);
        }
    }

    [TestClass]
    public class AccountTest
    {
        [TestMethod]
        public void AccountCreate()
        {
            // Setup test variables
            decimal testAmount = 1000;
            var testUserName = "Test User";

            // Setup test objects
            var testUser = new Person(testUserName);
            var testMoney = new Money(testAmount);
            var testAccount = new Account(testMoney, testUser);

            // Perform test
            Assert.AreEqual(testAccount.Money.Value, testMoney.Value);
        }
    }

    [TestClass]
    public class BankTest
    {

        private const string _testBankName = "DNB";
        private const string _testPerson1Name = "Jacob Tørring";
        private const string _testPerson2Name = "Erik Ormevik";
        private const decimal _testMoneyAccount1Amount = 1000;
        private const decimal _testMoneyAccount2Amount = 1000;

        [TestMethod]
        public void BankConstructor()
        {
            // Setup test objects
            var testBank = new Bank(_testBankName);

            // Assert
            Assert.AreEqual(_testBankName, testBank.BankName);
        }

        [TestMethod]
        public void BankCreateAccountPass()
        {
            // Setup test variables
            decimal testMoneyPersonAmount = 1600;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testMoneyPerson = new Money(testMoneyPersonAmount);
            var testAccount = new Account(testMoneyAccount, testPerson);

            // Perform test
            testPerson.Money = testMoneyPerson;
            testBank.CreateAccount(testPerson, testMoneyAccount);

            // Assert
            int testPersonAccountsIndex = testPerson.Accounts.Length - 1;

            // The new account has been created and added to person
            Assert.AreEqual(
                testAccount.Money.Value,
                testPerson.Accounts[testPersonAccountsIndex].Money.Value
                );

            // The new account contains the amount deposited
            Assert.AreEqual(
                _testMoneyAccount1Amount,                                   // The value to deposit
                testPerson.Accounts[testPersonAccountsIndex].Money.Value    // The value contained in the person's newest account
                );

            // The person's savings has been reduced accordingly
            Assert.AreEqual(
                testPerson.Money.Value,                                     // The current value of personal savings
                testMoneyPersonAmount - testPerson.Accounts[0].Money.Value  // The personal savings before creation - the value
                );                                                          // in person's newest account
        }

        [TestMethod]
        [ExpectedException(typeof(ArgumentException))]
        public void BankCreateAccountNegativeValue()
        {
            // Setup test variables
            decimal testMoneyPersonAmount = 1600;
            decimal testMoneyAccountAmount = -1000;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyAccount = new Money(testMoneyAccountAmount);
            var testMoneyPerson = new Money(testMoneyPersonAmount);

            // Perform test
            testPerson.Money = testMoneyPerson;

            // Assert
            // Expected to throw ArgumentException
            testBank.CreateAccount(testPerson, testMoneyAccount);
        }

        [TestMethod]
        [ExpectedException(typeof(ArgumentException))]
        public void BankCreateAccountInsufficientFunds()
        {
            // Setup test variables
            decimal testMoneyPersonAmount = 800;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testMoneyPerson = new Money(testMoneyPersonAmount);

            // Perform test
            testPerson.Money = testMoneyPerson;

            // Assert
            // Expected to throw ArgumentException
            testBank.CreateAccount(testPerson, testMoneyAccount);
        }

        [TestMethod]
        public void BankGetAccountsForCustomer()
        {
            // Setup test variables
            decimal testMoneyWithdrawAmount = 300;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testMoneyWithdraw = new Money(testMoneyWithdrawAmount);
            var testAccount = new Account(testMoneyAccount, testPerson);

            // Assert array is empty
            Assert.AreEqual(testBank.GetAccountsForCustomer(testPerson).Length, 0);

            // Perform test
            testPerson.AddAccounts(testAccount);

            // Assert account in array
            int testPersonAccountsIndex = testPerson.Accounts.Length - 1;

            Assert.AreEqual(
                testAccount, 
                testBank.GetAccountsForCustomer(testPerson)[testPersonAccountsIndex]
                );
        }

        [TestMethod]
        public void BankAccountWithdrawPass()
        {
            // Setup test variables
            decimal testMoneyWithdrawAmount = 300;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testMoneyWithdraw = new Money(testMoneyWithdrawAmount);
            var testAccount = new Account(testMoneyAccount, testPerson);

            // Perform test
            testBank.Withdraw(testAccount, testMoneyWithdraw);

            // Assert
            Assert.AreEqual(
                testAccount.Money.Value, 
                _testMoneyAccount1Amount - testMoneyWithdrawAmount
                );
        }

        [TestMethod]
        [ExpectedException(typeof(ArgumentException))]
        public void BankAccountWithdrawNegativeValue()
        {
            // Setup test variables
            decimal testMoneyWithdrawAmount = -300;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testMoneyWithdraw = new Money(testMoneyWithdrawAmount);
            var testAccount = new Account(testMoneyAccount, testPerson);

            // Perform test
            // Expected to throw ArgumentException
            testBank.Withdraw(testAccount, testMoneyWithdraw);
        }

        [TestMethod]
        [ExpectedException(typeof(ArgumentException))]
        public void BankAccountWithdrawInsufficientFunds()
        {
            // Setup test variables
            decimal testMoneyWithdrawAmount = 3000;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testMoneyWithdraw = new Money(testMoneyWithdrawAmount);
            var testAccount = new Account(testMoneyAccount, testPerson);

            // Perform test
            // Expected to throw ArgumentException
            testBank.Withdraw(testAccount, testMoneyWithdraw);

        }

        [TestMethod]
        public void BankAccountDepositPass()
        {
            // Setup test variables
            decimal testMoneyDepositAmount = 300;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyDeposit = new Money(testMoneyDepositAmount);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testAccount = new Account(testMoneyAccount, testPerson);

            // Perform test
            testBank.Deposit(testAccount, testMoneyDeposit);

            // Assert
            Assert.AreEqual(
                testAccount.Money.Value,
                _testMoneyAccount1Amount + testMoneyDepositAmount
                );
        }

        [TestMethod]
        [ExpectedException(typeof(ArgumentException))]
        public void BankAccountDepositNegativeValue()
        {
            // Setup test variables
            decimal testMoneyDepositAmount = -300;

            // Setup test objects
            var testBank = new Bank(_testBankName);
            var testPerson = new Person(_testPerson1Name);
            var testMoneyDeposit = new Money(testMoneyDepositAmount);
            var testMoneyAccount = new Money(_testMoneyAccount1Amount);
            var testAccount = new Account(testMoneyAccount, testPerson);

            // Perform test
            // Expected to throw ArgumentException
            testBank.Deposit(testAccount, testMoneyDeposit);
        }

        [TestMethod]
        public void BankAccountTransferPass()
        {
            // Setup test variables
            decimal testMoneyTransferAmount = 300;

            // Setup test objects
            var testBank = new Bank(_testBankName);

            var testPerson1 = new Person(_testPerson1Name);
            var testPerson2 = new Person(_testPerson2Name);

            var testMoneyAccount1 = new Money(_testMoneyAccount1Amount);
            var testMoneyAccount2 = new Money(_testMoneyAccount2Amount);
            var testMoneyTransfer = new Money(testMoneyTransferAmount);

            var testAccount1 = new Account(testMoneyAccount1, testPerson1);
            var testAccount2 = new Account(testMoneyAccount2, testPerson2);

            // Perform test
            // Transfer FROM testAccount1 TO testAccount2 WITH testMoneyTransfer
            testBank.Transfer(testAccount1, testAccount2, testMoneyTransfer);

            // Assert
            Assert.AreEqual(
                testAccount1.Money.Value,
                _testMoneyAccount1Amount - testMoneyTransferAmount
                );

            Assert.AreEqual(
                testAccount2.Money.Value,
                _testMoneyAccount1Amount + testMoneyTransferAmount
                );
        }

        [TestMethod]
        [ExpectedException(typeof(ArgumentException))]
        public void BankAccountTransferNegativeValue()
        {
            // Setup test variables
            decimal testMoneyTransferAmount = -200;

            // Setup test objects
            var testBank = new Bank(_testBankName);

            var testPerson1 = new Person(_testPerson1Name);
            var testPerson2 = new Person(_testPerson2Name);

            var testMoneyAccount1 = new Money(_testMoneyAccount1Amount);
            var testMoneyAccount2 = new Money(_testMoneyAccount2Amount);
            var testMoneyTransfer = new Money(testMoneyTransferAmount);

            var testAccount1 = new Account(testMoneyAccount1, testPerson1);
            var testAccount2 = new Account(testMoneyAccount2, testPerson2);

            // Perform test
            // Transfer FROM testAccount1 TO testAccount2 WITH testMoneyTransfer
            // Expected to throw ArgumentException
            testBank.Transfer(testAccount1, testAccount2, testMoneyTransfer);
        }

        [TestMethod]
        [ExpectedException(typeof(ArgumentException))]
        public void BankAccountTransferInsufficientFunds()
        {
            // Setup test variables
            decimal testMoneyTransferAmount = 2000;

            // Setup test objects
            var testBank = new Bank(_testBankName);

            var testPerson1 = new Person(_testPerson1Name);
            var testPerson2 = new Person(_testPerson2Name);

            var testMoneyAccount1 = new Money(_testMoneyAccount1Amount);
            var testMoneyAccount2 = new Money(_testMoneyAccount2Amount);
            var testMoneyTransfer = new Money(testMoneyTransferAmount);

            var testAccount1 = new Account(testMoneyAccount1, testPerson1);
            var testAccount2 = new Account(testMoneyAccount2, testPerson2);

            // Perform test
            // Transfer FROM testAccount1 TO testAccount2 WITH testMoneyTransfer
            // Expected to throw ArgumentException
            testBank.Transfer(testAccount1, testAccount2, testMoneyTransfer);
        }
    }
}

Answer 1

A simplification could be made if both, Persons and Accounts would implement a common interface

public interface IFund
{
    Money Fund { get; }
}

This would allow you to use the same test for both

private bool HasSufficientFounds(IFund fund, Money amount)
{
    ...
}

Now you can test

if (HasSufficientFounds(person, amount))

or

if (HasSufficientFounds(account, amount))

---

Unit tests. You could create a parameterizable and reusable test setup (I let you figure out the exact details):

private class TestSetup
{
    public TestSetup (decimal person1Fund = 0m, decimal person2Fund = 0m,
                      decimal account1Fund = 0m, decimal account2Fund = 0m,
                      decimal transferAmount = 0m)
    {
        Bank = new Bank ("First Bank");
        Person1 = new Person ("John Smith") { Fund = new Money(person1Fund) };
        Person2 = new Person ("Sue Miller") { Fund = new Money(person2Fund) };
        Account1 = ...
        ...
    }

    public Bank Bank { get; set; }
    public Person Person1 { get; set; }
    public Person Person2 { get; set; }
    public Account Account1 { get; set; }
    public Account Account2 { get; set; }
    public Money Transfer { get; set; }
}

Since the parameters are optional, you need to pass only those you need for a test.

Answer 2

You don't need a backing field

public string BankName { get; set; }

Answer 3

Some thoughts on the exception handling/validation part:

For methods that validate input data, it seems that the choice you have is between either returning a boolean based on whether or not the input is valid or by throwing an Exception if the input was found to be invalid and returning without a value if the input was found to be valid.

Instead of mixing both approaches, like you're doing now, you might consider sticking with one of them and naming the methods accordingly. For example, _validPersonWithdrawTransaction becomes _validatePersonWithdrawTransaction. A method with that name would not be expected to return anything at all and you could just let that Exception bubble up to whereever you want to handle it instead of relying on the return value of the function as well.