3
\$\begingroup\$

I have been working on this:

Design a class for a bank database. The database should support the following operations:

❍ deposit a certain amount into an account

❍ withdraw a certain amount from an account

❍ get the balance (i.e., the current amount) in an account

❍ transfer an amount from one account to another

The amount in the transactions is a value of type double. The accounts are identified by instances of the class Account that is in the package com.megabankcorp.records. The database class should be placed in a package called com.megabankcorp.system. The deposit, withdraw, and balance operations should not have any implementation, but allow subclasses to provide the implementation. The transfer operation should use the deposit and withdraw operations to implement the transfer. It should not be possible to alter this operation in any subclass, and only classes within the package com.megabankcorp.system should be allowed to use this operation. The deposit and withdraw operations should be accessible in all packages. The balance operation should only be accessible in subclasses and classes within the package com.megabankcorp.system.

And this is the implementation:

Bank.java

package com.megabankcorp.system;

import java.util.List;

import com.megabankcorp.records.Account;

public abstract class Bank {

    public abstract void depositAmount(Account account, double amount);
    public abstract void withdrawAmount(Account account, double amount);
    protected abstract double currentBalance(Account account);

    List<Account> accounts; //Can be more than one account.

    boolean transferAmount(Bank bank, Account transferFrom, Account transferTo, double amount) {
        boolean isTransferred = false;
        if (transferFrom.getAccountNumber() == transferTo.getAccountNumber()) {
            System.out.println("Can not transfer in your own account.");
        } else if (transferFrom.getCurrentAmount() < amount) {
            System.out.println("You have insufficient funds.");
        } else {
            bank.depositAmount(transferTo, amount);
            bank.withdrawAmount(transferFrom, amount);
            isTransferred = true;
        }
        return isTransferred;
    }
}

ABC.java

package com.megabankcorp.system;

import java.util.ArrayList;

import com.megabankcorp.records.Account;

public class ABC extends Bank {

    public ABC () {
        this.accounts = new ArrayList<Account>();
    }

    @Override
    public void depositAmount(Account account, double amount) {
        account.setCurrentAmount(account.getCurrentAmount() - amount);
    }

    @Override
    public void withdrawAmount(Account account, double amount) {
        account.setCurrentAmount(account.getCurrentAmount() - amount);
    }

    @Override
    protected double currentBalance(Account account) {
        return account.getCurrentAmount();
    }



}

Account.java

package com.megabankcorp.records;


public class Account {

    private int accountNumber;
    private double currentAmount;

    public Account (int accountNumber) {
        this.accountNumber = accountNumber;
    }

    public int getAccountNumber() {
        return accountNumber;
    }

    public double getCurrentAmount() {
        return currentAmount;
    }

    public void setCurrentAmount(double amount) {
        this.currentAmount = amount;
    }
}

Please suggest the best practice possible for this problem. Any suggestions and ideas are welcome.

\$\endgroup\$
  • 1
    \$\begingroup\$ This is an elementary school problem. It's like asking for best practices to add 2 and 2 together. \$\endgroup\$ – PM 77-1 Nov 21 '14 at 20:50
  • \$\begingroup\$ Is that really so? \$\endgroup\$ – D D Nov 21 '14 at 21:43
  • \$\begingroup\$ @PM77-1 Do you think this code is perfect then? \$\endgroup\$ – abuzittin gillifirca Nov 25 '14 at 8:01
  • 1
    \$\begingroup\$ It should be noted that for actual financial software, you shouldn't use double, instead use BigDecimal. \$\endgroup\$ – esote Jul 7 '18 at 2:01
3
\$\begingroup\$

Emphases added in quotes below.

Design a class for a bank database.

Write one class. The rest should be stubbed just enough to make the main thing you are writing compile. Especially there should not be a class ABC.

Design a class for a bank database.

If it is called "bank database" in the requirements, why the class is named Bank.

It should not be possible to alter this operation in any subclass

They ask you to make transfer method final, which you did not.

transfer an amount from one account to another

There is "an amount", "from one account", "to another", where did you get the Bank bank argument in the transfer method. If I'm invoking a method on an instance, I do not expect to pass the object to itself; use this instead.

Also there is no mention of that method returning anything. Methods that indicate actions or commands etc should declare return type void, whenever possible.

Use appropriate exceptions instead of returning error codes. Change the types of exceptions as you learn more about exception handling:

if (transferFrom.getAccountNumber() == transferTo.getAccountNumber())
    throw new IllegalArgumentException("Can not transfer in your own account.");

if (transferFrom.getCurrentAmount() < amount)
    throw new IllegalStateException("You have insufficient funds.");

this.depositAmount(transferTo, amount);
this.withdrawAmount(transferFrom, amount);

The requirement says "transfer" "from one account" "to another [account]", but you transfer transferFrom transferTo, that doesn't read well at all. Rename them fromAccount and toAccount.

There is nothing mentioned about .getAccountNumber(), or about account numbers being comparable by ==. It probably should be something like fromAccount.equals(toAccount). And what two accounts being equal means should be defined in Account.equals() (which you needn't write yet, which is always something good). Also requirement says:

The accounts are identified by instances of the class Account

Account class appears to be a --badly named-- identifier. We don't know that balance is in it, and not in some other class "identified by ... Account", say AccountRecord. The gist is you don't know, and you don't need to know. (Not needing something is better that needing something. Therefore not needing to know is better than needing to know. OOP is focused a lot on reducing the amount of things you need to know.)

Also just lose the List<Account> accounts; field. There is nothing about it in the requirements.

\$\endgroup\$
  • \$\begingroup\$ Really, thanks a lot. I am so glad that you took time to read my problem and help me, that's really helping. \$\endgroup\$ – D D Nov 25 '14 at 9:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.