Erronous output from program using derived classes. Not for the light of heart

Note: I apologize if this program is painful to look at. this is my first program using derived classes. I plan on submitting it to code review after I get it to do what I want so I can recieve tips on how it could have been designed better. Feel free to provide those tips here but my main intention for this post is to get it to work correctly.

This program processes withdrawals and deposits for bank accounts and then applies an interest rate.

the problem: For some reason it seems like the value of checkingBalance gets stuck and does not update to a new value throughout the rest of the loop when processing the accounts of class type "checkingAccount". It is odd because the same loop is used to process the other derived class "savingsAccount" and it works fine and one of accounts of type "checkingAccount" even seems to have been processed correctly so I am completely bamboozled as to what is going on.

If anyone could lend a hand and help me resolve this issue it would be greatly appreciated. Please do not hesitate to ask me to specify anything, I will respond asap.

the data being read:

The output:

The code I suspect the issue to be in:

//This loop is exited early:

 for (int j = 0; j < transactionNum; j++)
            {
                inFile >> transactionTypeTemp >> amountTemp;
                inFile.get(discard);
                ba.applyTransaction(accountType, transactionTypeTemp, amountTemp, j);
            }

//when something in here occurs?
// c = j from loop

double checkingAccount:: applyTransaction(char transactionTypeTemp, int amountTemp, int c, double checkingBalance)
{
  if (transactionTypeTemp == 'D')
        {
            checkingBalance = checkingBalance + amountTemp;
        }
  else if (transactionTypeTemp == 'W')
        {
            if (checkingBalance < amountTemp)
            {
            cout << "error: transaction number " << c + 1 << " never occured due to insufficent funds." << endl;
            }
            else
            {
                checkingBalance = checkingBalance - amountTemp;
                if(checkingBalance < minimumBalance) //if last transaction brought the balance below minimum balance
                {
                    checkingBalance = (checkingBalance - serviceCharge); //apply service charge
                }
            }
        }

        return checkingBalance;
}

the whole program:

//header file
#include <iostream>
#include <fstream>


using namespace std;

class bankAccount
{
    public:
    bankAccount();
    void setAccountInfo(int accountNumTemp, double balanceTemp);
    void prePrint(char accountType);
    void applyTransaction(char accountType, char transactionTypeTemp, int amountTemp, int j);
    void applyInterest(char accountType);
    void postPrint();

    private:
    int accountNumber;
    double balance;
};

class checkingAccount: public bankAccount
{
    public:
    void prePrint(int accountNumber, char accountType, double checkingBalance);
    checkingAccount();
    double  applyTransaction(char transactionTypeTemp, int amountTemp, int c, double    checkingBalance);
    double  applyInterest(double checkingBalance);

    private:
    float interestRate;
    int minimumBalance;
    float serviceCharge;

};

class savingsAccount: public bankAccount
{
    public:
    void prePrint(int savingsAccountNumber, char accountType, double savingsBalance);
    savingsAccount();
    double applyTransaction(char transactionTypeTemp, int amountTemp, int c, double savingsBalance);
    double applyInterest(double checkingBalance);

    private:
    float interestRate;
};

//class implementation .cpp file

bankAccount:: bankAccount()
{
    accountNumber = 0;
    balance = 0;
}

void bankAccount:: setAccountInfo(int accountNumTemp, double balanceTemp)
{
    accountNumber = accountNumTemp;
    balance = balanceTemp;
}

void bankAccount:: prePrint(char accountType)
{
    if(accountType == 'C')
    {
        int checkingAccountNumber = accountNumber;
        double checkingBalance = balance;
        checkingAccount ca;
        ca.prePrint(checkingAccountNumber, accountType, checkingBalance);
    }
    else if (accountType == 'S')
    {
        int savingsAccountNumber = accountNumber;
        double savingsBalance = balance;
        savingsAccount sa;
        sa.prePrint(savingsAccountNumber, accountType, savingsBalance);
    }


}

void bankAccount:: applyTransaction(char accountType, char transactionTypeTemp, int amountTemp, int j)
{
    int c;
    c = j;
    double checkingBalance, savingsBalance;
    checkingAccount ca;
    savingsAccount sa;

        if (accountType == 'C')
        {
            checkingBalance = balance;
            balance = ca.applyTransaction(transactionTypeTemp, amountTemp, c, checkingBalance);
        }
        else if (accountType == 'S')
        {
            savingsBalance = balance;
            balance = sa.applyTransaction(transactionTypeTemp, amountTemp, c, savingsBalance);
        }

}

void bankAccount:: applyInterest(char accountType)
{
    double checkingBalance, savingsBalance;
    checkingAccount ca;
    savingsAccount sa;

    if (accountType == 'C')
    {
    checkingBalance = balance;
    balance = ca.applyInterest(checkingBalance);
    }
    else if (accountType == 'S')
    {
    savingsBalance = balance;
    balance = sa.applyInterest(savingsBalance);
    }


}

void bankAccount:: postPrint()
{
   cout << "Balance after processing: " << balance << endl;
}

checkingAccount:: checkingAccount()
{
    interestRate = .02;
    minimumBalance = 500;
    serviceCharge = 20;
}

void checkingAccount:: prePrint(int checkingAccountNumber, char accountType, double checkingBalance)
{
    cout << "Account Number:" << checkingAccountNumber << " account type:" << accountType << " Starting Balance:" << checkingBalance << endl;
}

double checkingAccount:: applyTransaction(char transactionTypeTemp, int amountTemp, int c, double checkingBalance)
{
  if (transactionTypeTemp == 'D')
        {
            checkingBalance = checkingBalance + amountTemp;
        }
  else if (transactionTypeTemp == 'W')
        {
            if (checkingBalance < amountTemp)
            {
            cout << "error: transaction number " << c + 1 << " never occured due to insufficent funds." << endl;
            }
            else
            {
                checkingBalance = checkingBalance - amountTemp;
                if(checkingBalance < minimumBalance) //if last transaction brought the balance below minimum balance
                {
                    checkingBalance = (checkingBalance - serviceCharge); //apply service charge
                }
            }
        }

        return checkingBalance;
}

double checkingAccount:: applyInterest(double checkingBalance)
{
    checkingBalance = (checkingBalance + (checkingBalance * interestRate));
    return checkingBalance;
}
savingsAccount:: savingsAccount()
{
    interestRate = .04;
}


void savingsAccount:: prePrint(int savingsAccountNumber, char accountType, double savingsBalance)
{
    cout << "Account Number:" << savingsAccountNumber << " account type:" << accountType << " Starting Balance:" << savingsBalance << endl;
}

double savingsAccount:: applyTransaction(char transactionTypeTemp, int amountTemp, int c, double savingsBalance)
{
  if (transactionTypeTemp == 'D')
        {
            savingsBalance = savingsBalance + amountTemp;
        }
  else if (transactionTypeTemp == 'W')
        {
            if (savingsBalance < amountTemp)
            {
            cout << "error: transaction number" << c + 1 << " never occured due to insufficent funds." << endl;
            }
            else
            {
                savingsBalance = savingsBalance - amountTemp;     //apply transaction
            }
        }

        return savingsBalance;
}

double savingsAccount:: applyInterest(double savingsBalance)
{
    savingsBalance = (savingsBalance + (savingsBalance * interestRate));
    return savingsBalance;
}

//main .cpp file
int main()
{
    ifstream inFile;
    int numberOfAccounts, accountNumTemp, transactionNum, amountTemp;
    double balanceTemp;
    char discard, accountType, transactionTypeTemp;
    bankAccount ba;

    cout << "Processing account data..." << endl;

    inFile.open("Bank.txt");

    if (!inFile)
    {
        for  (int a = 0; a < 20; a++)
            cout  << endl;
        cout << "Cannot open the input file."
             << endl;
            return 1;
    }

    inFile >> numberOfAccounts;
    inFile.get(discard);

    for (int i = 0; i < numberOfAccounts; i++)
    {
            inFile >> accountNumTemp >> accountType >> balanceTemp >> transactionNum;
            inFile.get(discard);
            ba.setAccountInfo(accountNumTemp, balanceTemp);
            ba.prePrint(accountType);

            for (int j = 0; j < transactionNum; j++)
            {
                inFile >> transactionTypeTemp >> amountTemp;
                inFile.get(discard);
                ba.applyTransaction(accountType, transactionTypeTemp, amountTemp, j);
            }
            ba.applyInterest(accountType);
            ba.postPrint();
    }


    inFile.close();

    return 0;
}

Here is the input in copy-and-pastable form so that others don't have to type it in:

6
35231 C 500 3
W 250
W 200
W 100
46728 S 2700 2
D 250
W 100
87324 C 500 3
D 300
W 100
W 150
79873 S 800 0
89932 C 3000 2
开发者_C百科W 1000
W 1500
98322 C 750 6
D 50
W 75
W 100
W 25
W 30
W 75

Hmm...doing a quick compile and run with your data, I get the following output:

Processing account data...
Account Number:35231 account type:C Starting Balance:500
error: transaction number 3 never occured due to insufficent funds.
Balance after processing: 10.2
Account Number:46728 account type:S Starting Balance:2700
Balance after processing: 2964
Account Number:87234 account type:C Starting Balance:500
Balance after processing: 561
Account Number:79873 account type:S Starting Balance:800
Balance after processing: 832
Account Number:89832 account type:C Starting Balance:3000
Balance after processing: 510
Account Number:98322 account type:C Starting Balance:750
Balance after processing: 484.5

Most of this looks fairly reasonable to me. Doing a quick addition in my head, the "insufficient funds" seems reasonable for the first account since you're giving it an initial balance of 500, and the withdrawals add up to 550.

To put it as nicely as I know how, I don't think I'd write the code the same way you have, but the output I'm getting doesn't seem to have nearly the same problems you've shown. That could be from undefined behavior (e.g., using an uninitialized variable), but I haven't spent enough time looking through the code to be sure. Personally, I think I'd rather spend my time on a different design than on finding possible bugs in the code as it stands right now though.

Edit: As far as re-designing the code goes, when you have code that needs to be implemented different in the two derived classes, you want to use a virtual function, and implement it appropriately in each derived class. Instead of creating bank accounts as local variables in each member function, and creating a new bank account object in each, I'd create one account object as I'm reading in the data that defines an account, and then use that account object for all the transactions on that object.

It also seems to be open to considerable question whether you really even need a base class and derived classes at all. I didn't look too carefully, but it looks to me like the behavior of the two types of accounts is pretty much (if not completely) identical. The only difference I saw was that a savings account doesn't allow a withdrawal into negative numbers, where a checking account does allow such withdrawals, but charges a service charge. Though you don't model it, if a checking account balance drops below some point, I'm pretty sure they'll start to bounce checks as well.

That being the case, it looks to me like you could model it all as one type of account, but with two minimum levels, and a fee associated with violating each. The first level means the transaction is allowed, but a fee is charged. The second means the transaction is not allowed.

For a savings account, both levels and fees are set to $0.0. For a checking account, the "charge fee" level is $0.0, and the "refuse transaction" level is (say) -$50. For both, the fee is, say, $50.

Ick.

The code doesn't actually seem incorrect, therefore you might need to look at the data - did you move this file using ftp and mess up the line endings for instance.

Refactoring this... For a start it is just horrible to be writing your own version of virtual using if/else. Second updating class variables by passing them to member functions and updating as a result confuses the process.

Starting from BankAccount ba; you are re using this for each account, it makes the rest of the program more difficult to write and read. Use a Factory/Lookup method instead.

for (int i = 0; i < numberOfAccounts; i++)
{
        inFile >> accountNumTemp >> accountType >> balanceTemp >> transactionNum;
        inFile.get(discard);
        bankAccount &ba = bankAccount::getAccount(accountType, accountNumTemp, balanceTemp);

Without making a full blow factory it might go something like

class bankAccount
{
    protected:
    bankAccount(int accountNumTemp, double balanceTemp);

    public:
    static bankAccount& getAccount( char accountType, int accountNumTemp, double balanceTemp){
        switch(accountType)
          case 'C':
            return checkingAccount(accountNumTemp, balanceTemp);
          case 'S':
            return savingsAccount(accountNumTemp, balanceTemp);
          default:
            throw exception();
        }

    }
    void prePrint();
    void postPrint();

    virtual string type() = 0;
    virtual void applyTransaction( char transactionTypeTemp, int amountTemp, int j) = 0;
    virtual void applyInterest() =0;

    protected:
    int accountNumber;
    double balance;
};

Then as an example the child classes would change like

class checkingAccount: public bankAccount
{
    public:
    checkingAccount(int accountNumTemp, double balanceTemp)
        : bankAccount(accountNumTemp, balanceTemp)
    { // these are currently unchanged, could probably move to static
        interestRate = .02;
        minimumBalance = 500;
        serviceCharge = 20;
    }

    void  applyTransaction(char transactionTypeTemp, int amountTemp, int c );
    void  applyInterest();

    private:
    float interestRate;
    int minimumBalance;
    float serviceCharge;

};

void bankAccount::prePrint()
{
    cout << "Account Number:" << accountNumber 
      << " account type:" << type() 
      << " Starting Balance:" << balance << endl;
}

string checkingAccount::type(){ return "C" }

void checkingAccount:: applyTransaction(char transactionTypeTemp, int amountTemp, int c )
{
  if (transactionTypeTemp == 'D')
        {
            balance = balance + amountTemp;
        }
  else if (transactionTypeTemp == 'W')
        {
            if (balance < amountTemp)
            {
            cout << "error: transaction number " << c + 1 << " never occured due to insufficent funds." << endl;
            }
            else
            {
                balance = balance - amountTemp;
                if(balance < minimumBalance) //if last transaction brought the balance below minimum balance
                {
                    balance = (balance - serviceCharge); //apply service charge
                }
            }
        }
}

void checkingAccount:: applyInterest()
{
    balance = (balance + (balance * interestRate));
}

Well, your output is correct. Can you provide some suggestions on where I should start with better design?

I'd start with

• unit tests
• a debugger

and follow up with

• valgrind

Update

I just did step 3 (valgrind), and it just works, providing the same output as Jerry:

Processing account data...
Account Number:35231 account type:C Starting Balance:500
error: transaction number 3 never occured due to insufficent funds.
Balance after processing: 10.2
Account Number:46728 account type:S Starting Balance:2700
Balance after processing: 2964
Account Number:87324 account type:C Starting Balance:500
Balance after processing: 561
Account Number:79873 account type:S Starting Balance:800
Balance after processing: 832
Account Number:89932 account type:C Starting Balance:3000
Balance after processing: 510
Account Number:98322 account type:C Starting Balance:750
Balance after processing: 484.5

Note that this is with this exact input (I found the input to be rather sensitive to e.g. case changes):

6
35231 C 500 3
W 250
W 200
W 100
46728 S 2700 2
D 250
W 100
87324 C 500 3
D 300
W 100
W 150
79873 S 800 0
89932 C 3000 2
W 1000
W 1500
98322 C 750 6
D 50
W 75
W 100
W 25
W 30
W 75