HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

OOP bank database

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bankdatabaseoop

Problem

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 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() < amou

Solution

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 accounts; field. There is nothing about it in the requirements.

Code Snippets

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

Context

StackExchange Code Review Q#70523, answer score: 3

Revisions (0)

No revisions yet.