Is there a better / shorter way to do this?

[will808]

Well, I finally bit the bullet and bought some professional training - it's great fun, but what I've covered so far is limited to the fundamentals. So, I've got to create an abstract Bank Account class, and then create subclasses that form a current account and a savings account.

Everything works fine, but I'm wondering if there is a shorter way for my Current account withdrawal() method. The constraints are:

1) Withdrawals must be within the overdraft limit (set by me at -200).
2) Regardless of the overdraft limit, the Current account overdrawn balance must not exceed the balance of the savings account (ie, if there is ony £150 in the savings account, you can only go £150 overdrawn).
3) A fixed fee must be charged for invoking the overdraft.

Originally, I had it set so that every withdrawal that gave a negative balance invoked the overdraftFee. Then I decided that was unfair (even though it's my bank, so I can do as I please ), and decided that the overdraftFee should only be charged when the account goes from a positive balance into a negative one. In other words, it only gets charged once (unless you go back to a positive balance, and then back into negative - a one time fee would require some kind of date / overdraftFirstInvoked method, and that's beyond the scope of what I have to do.

So, my "be nice to the customer" routine has trebled the amount of code needed for the withdraw() method. It all works, but I was wondering if there was a shorter / neater / better way of doing this while sticking to the fundamentals. Sorry for the long post, but I wanted to explain exactly where I am and what is expected of me:

public boolean withdraw(double amount) {

//if we're staying positive, it's ok to withdraw the amount
if(balance - amount >= 0) {
balance -= amount;
System.out.print("Withdrawal successful. ");
printBalance();
return true;
}


//else if amount would exceed overdraft limit, fail
else if(balance - amount < overdraftLimit) {
System.out.print("Transaction denied - balance would exceed overdraft limit");
printBalance();
return false;
}


//else if amount would exceed the (negative version of) savings balance, fail
else if (balance - amount < (SavingsAccount.balance * -1)) {
System.out.print("Transaction denied - balance would exceed available fubds in Savings Account");
printBalance();
return false;
}

// we're going legally negative (i.e. above both overdraft limit & savings balance)
else {

//if starting from a positive, we'll incur an overdraft fee
if(balance >= 0) {
//if withdrawing takes us within "fee" of the overdraft limit, fail
if(balance - amount < overdraftLimit + overdraftFee) {
System.out.print("Transaction denied - withdrawal amount plus overdraft fee would exceed overdraft limit");
printBalance();
return false;
}


if(balance - (amount + overdraftFee) < SavingsAccount.balance *-1) {
System.out.print("Transaction denied - withdrawal plus overdraft fee would exceed Savings Account balance");
return false;

}
//apply fee and allow withdrawal
else {
balance -= (amount + overdraftFee);
System.out.print("Withdrawal successful, overdraft fee invoked. ");
printBalance();
return true;
}
}
//we're already negative, going more negative, but don't incur fee (as starting with negative already)
else {
balance -= amount;
printBalance();
return true;
}

}

}


Many thanks!

[sjalle]

Since you have a set of six messages to be chosen from, I don't think a
further 'boil-down' of the code would pay...
Also, voluminous code that is reduced to become more 'clever' can sometimes
reduce the readability of the code, and that can make future maintenance
a real pain...

public boolean withdraw(double amount) {
    double newBalance=balance - amount;
    String msg=null;
    boolean approved=true; // be an optimist
    //if we're staying positive, it's ok to withdraw the amount
    if (newBalance >= 0) {
      msg="Withdrawal successful.";
    } else if (newBalance < overdraftLimit) {
      //else if amount would exceed overdraft limit, fail
      msg="Transaction denied - balance would exceed overdraft limit";
      approved=false;
    } else if (newBalance < (SavingsAccount.balance * -1.0d)) {
      //else if amount would exceed the (negative version of) savings balance, fail
      msg="Transaction denied - balance would exceed available funds in Savings Account";
      approved=false;
    } else {
      // we're going legally negative (i.e. above both overdraft limit & savings balance)
      //if starting from a positive, we'll incur an overdraft fee
      if (balance >= 0) {
        //if withdrawing takes us within "fee" of the overdraft limit, fail
        if (newBalance < overdraftLimit + overdraftFee) {
          msg="Transaction denied - withdrawal amount plus overdraft fee would exceed overdraft limit";
          approved=false;
        }
        if (balance - (amount + overdraftLimit) < SavingsAccount.balance * -1.0d) {
          msg="Transaction denied - withdrawal plus overdraft fee would exceed Savings Account balance";
          approved=false;
        } else { //apply fee and allow withdrawal
          newBalance=(balance -= (amount + overdraftFee));
          msg="Withdrawal successful, overdraft fee invoked. ";
        }
      }
    }
    if (msg != null) System.out.println(msg);
    if (approved) {
      balance=newBalance;
      printBalance();
    }
    return approved;  
  }

[will808]

That's great - thanks for taking a look SJalle. It just seemed a lot of code for a fairly straightforward thing, but I agree with you about keeping it legible. Thanks again