Trying to create OO based bank app

[Tushar Goel]

Campbell...Really? It is what you wrote earlier minus the == true bit. Well, as near as I could remember it, anyway.

Yes it is

Campbell....I thought we had discussed reopening a closed account, in which case that method should throw an Exception.

Yes we did but i put this check on another methods instead of setActivate() method itself.

Junilu
Don't get down on yourself or apologize for not knowing any better.....................Here's a search that brings up a few, including JavaRanch's own style

Thanks for motivating and sharing the links. I read java ranch style guide and update my code accordingly. I have update setActivate method as below

	/**
	 * If account is not closed then this will set the activate or in-activate status of the account.
	 * 
	 * @param isActive   to change bank account status. true to make active
	 * 					 false to make account inactive.
     * @throws IllegalStateException   When trying to change the status of already closed 
     * 								   accounts
     */
	public void setActivate(boolean isActive) throws IllegalStateException 
	{	
		testAccountAvailable();
		accountStatus = isActive  ? AccountStatus.OPEN : AccountStatus.INACTIVE ;
	}

[Tushar Goel]

I tried to make a penalty system:

Penalty will be applicable for 2 cases:
1) If minimum balance is not maintained and this checked on Quarterly basis. I will do this later.
2) If over withdraw happened.

       /**
	 * User can withdraw amount up to maximum over draft limit on non closed
	 * accounts only.
         * @param  transaction    Transaction object  which having information about
         *                        type of transaction and amount involved.
	 * @throws InsufficientFundException  When withdraw amount greater than 
	 *									  (available balance + maximumOverdraftAmount)									
         * @throws IllegalStateException      When trying to do withdraw on closed account
         */
	void doWithdraw(Transaction transaction)
			       throws InsufficientFundException,IllegalStateException 
	{
		testAccountAvailable();	
		testWithdrawAmountAgainstMaximumOverwithdraw(transaction);
        
		// if over withdraw required
	    if (compareAmount(transaction.getAmount(), balance)) {
			new PenaltyRule(penalty).apply(this);
		}	
	    
	    storeTransaction(transaction);
		balance = balance.subtract(transaction.getAmount());
	}

    /**
     * Check if amount to be withdraw is greater than available balance + 
     * maximum overdraft amount or not.
     * 
     * @param transaction     Transaction requested by the user
     * @throws InsufficientFundException  When amount to be withdraw > (available
     *                                    balance + maximum overdraft amount )
     */
	private void testWithdrawAmountAgainstMaximumOverwithdraw(Transaction transaction)
								throws InsufficientFundException {
		
		BigDecimal amount = balance.add(maximumOverdraftAmount);
		
		if (compareAmount(transaction.getAmount(), amount)) {
			throw new InsufficientFundException("Balance is insufficient for withdraw");	
		}
		
	}

PenaltyRule:

public class PenaltyRule implements AccountRule {

    private BigDecimal penaltyAmount;   
    
    /**
     * Initialize value
     * @param penaltyAmount
     */
	public PenaltyRule(BigDecimal penaltyAmount) {
		this.penaltyAmount = penaltyAmount;
	}

	
    /**
     * Reduce penalty amount from the user account 
     * @param  account    account on which penalty will be calculated
     */
	@Override
	public void apply(Account account) {  	
        BigDecimal balance = account.getBalance();
       	account.setBalance(balance.subtract(penaltyAmount));     
        setPenaltyTransaction(account);
	}

	/**
	 * This will store the penalty transaction to the list of the transaction
	 * performed by the account
	 * 
	 * @param  account user account in which transaction needs to be added
	 */
	private void setPenaltyTransaction(Account account) {
        account.storeTransaction(new Charges(penaltyAmount));	
	}
}

[Campbell Ritchie]

Surely the method should take the amount as an argument and be called on the Account object.
Would you have a static penalty amount in the Account class; different subclasses can have different penalties? That would make the penalty the same for all instance of a particular type of account.

[Tushar Goel]

ould you have a static penalty amount in the Account class; different subclasses can have different penalties?

I tried to have that implemented but in this case i always getting penalty value from parent class.

I tried to have simple example as well but result is same:

class A {
  static int x = 5;

  void display() {
    System.out.println(x);
  }
}

class B extends A {
  static int x = 10;

  public static void main(String... args) {
       new B().display();
  }
} 

It always print 5. So i can not use but i think i need to define static methods and have to override them

[Campbell Ritchie]

The static method is declared in class A so it uses the field in class A. Remember things static are not polymorphic. Their values are statically linked before run‑time. Things polymorphic (=instance methods) are linked dynamically at run‑time. I think it is because of static linking that they chose the keyword static.

[Tushar Goel]

then we cannot have static penalty amount in the Account class so that different subclasses can have different penalties because we defining all methods (deposit, withdraw etc)
or call (penalty or interest etc) from account class itself so it always uses value from the parent not subclasses.

[Campbell Ritchie]

Have you tried a Map<Class<?>, Penalty> ?

[Tushar Goel]

Thanks Campbell... I made below changes:

public interface Penalty {
	/**
	 * This will return the penalty
	 * @return Penalty amount
	 */
	BigDecimal getPenalty();

}

public class SavingAccountPenalty implements Penalty {
	@Override
	public BigDecimal getPenalty() {
		return BigDecimal.TEN;
	}
}

Account class I have removed other variables from the account constructor to check if implementation of penalty is correct. If its correct then other will be implemented in same way

public abstract class Account {

	private static final Map<Class<?>, Penalty> penaltyForEachTypeOfAccount = new HashMap<>();
       
        // other declaration........
        
	/**
	 * Initialize value
	 * @param balance     amount entered by user while opening an account
         * @param className class name for which account created
	 * @throws IllegalArgumentException   When amount <= 0
	 * @throws NullPointerException       when amount is null
	 */
	public Account(BigDecimal balance, Class<?> className)  throws............. {
		
        if (balance == null)
        {
        	throw new NullPointerException("amount can not be null");
        }else if (balance.compareTo(BigDecimal.ZERO) <= 0)
        {
        	throw new IllegalArgumentException("amount can not be 0 or less than 0");
        }
        this.penalty = penaltyForEachTypeOfAccount.get(className).getPenalty();;
    
        //other initialization
	}

	/**
	 * @return the penaltyForEachTypeOfAccount
	 */
	static Map<Class<?>, Penalty> getpenaltyMapForEachTypeOfAccount() {
		return penaltyForEachTypeOfAccount;
         }

         // other methods........
	}

SavingAccount One of the type of account

public class SavingAccount extends Account {
	static {
		Account.getpenaltyMapForEachTypeOfAccount().putIfAbsent(SavingAccount.class, new SavingAccountPenalty());
	}
	
	/**
	 * Initialize value
	 * @param balance
	 * @throws IllegalArgumentException
	 * @throws NullPointerException
	 */
	public SavingAccount(BigDecimal balance) 
	 		throws IllegalArgumentException, NullPointerException {
		super(balance, SavingAccount.class);
	}
}

[Campbell Ritchie]

What is the access of getPenaltyMap...? Is it package‑private? I presume only the Account classes are in that package.

[Tushar Goel]

Yes, it is package private. the package name is 'onlinebanking' and all classes except utility and test classes in it.

So should i initialize all other things like minimum balance , interest rate etc like this ?

[Campbell Ritchie]

I still don't understand why minimum balance can't be a class attribute.

[Tushar Goel]

because like Penalty , minimum balance is also variable. Each type of account having different minimum balance. Say Saving type of account have 504 minimum
but the Salary account have 0 minimum balance etc. (Actually in real life i am having both type of accounts and they have same amount of minimum balance)

[Campbell Ritchie]

And why can't those be static fields of the individual account classes?

[Junilu Lacar]

Have a look at Martin Fowler's analysis patterns for accounting: http://martinfowler.com/apsupp/accounting.pdf

It might help you see the different possibilities for organizing your code and concepts if you read through this article and Fowler's examples. One of the more interesting things about these patterns is that some things that you may think should be implemented as attributes of an object can actually be treated as "rules". For example, a minimum balance rule can be applied to some types of accounts, applied differently for others, and perhaps not at all for some.

[Tushar Goel]

Campbell Do you mean this?

    public class SavingAccountPenalty implements Penalty {  
        @Override  
        public static BigDecimal getPenalty() {  
            return BigDecimal.TEN;  
        }  
    }  

    public class SavingAccount extends Account {  
        private static BigDecimal penalty = SavingAccountPanalty.getPenalty();
        
        /** 
         * Initialize value 
         * @param balance 
         * @throws IllegalArgumentException 
         * @throws NullPointerException 
         */  
        public SavingAccount(BigDecimal balance)   
                throws IllegalArgumentException, NullPointerException {  
            super(balance, penalty, SavingAccount.class);  
        }  
    }  

Have a look at Martin Fowler's analysis patterns

Thanks for sharing the link Junilu, Yes, i am checking the same and get back to the post...

[Campbell Ritchie]

I haven't had the time to read that Martin Fowler link, but I never knew about it before.

Might be worth reading the Martin Fowler link and digesting it before doing anything else.

[Tushar Goel]

I found it to be little bit difficult to understand but i am still trying to understand what Martin wanted to convey, it will take some more time.. So, please be with me...

[Tushar Goel]

Hi, Sorry for delay. I am quite busy with personal and official commitment so could not able to spare much time.

Thanks Junilu, for the link it is very good. As per doc, i have re-modified my application... This looks like as per screenshot. Please suggest.