C++ Confusion: Bank Transaction Test

Discussion in 'Mac Programming' started by Josh, Apr 9, 2006.

  1. macrumors 68000

    Josh

    Joined:
    Mar 4, 2004
    Location:
    State College, PA
    #1
    Hi,

    I'm writing a small program that takes an input file such as this:
    Code:
    Checking Deposit    50.00
    Savings  Deposit    100.00
    Savings  Withdrawal  21.50
    Checking Transfer    42.68
    Checking Deposit     14.95
    Checking Withdrawal  25.00
    Savings  Deposit      1.25
    Checking Deposit     25.00
    
    then processes the info, adding or subtracting to the balance of the checking or savings account as needed.

    Ideally, the output should be similar to this:
    Code:
    Welcome to the Bank!
    
    Please enter your name: Mark
    
    Transaction            Amount   Savings   Checking
    --------------------   ----------  --------  -----------
    Checking Deposit       50.00      0.00     50.00 
    Savings  Deposit      100.00    100.00     50.00 
    Savings  Withdrawal   21.50     78.50     50.00
    Checking Transfer      42.68    121.18      7.32
    Checking Deposit       14.95    121.18     22.27
    Checking Withdrawal  25.00 *Insufficient Funds*
    Savings  Deposit         1.25    122.43     22.27
    Checking Deposit       25.00    122.43     47.27
    
    Thanks for visiting the bank Mark
    
    My program will consist of 3 additional boolean functions (titled: deposit, withdrawl, transfer) to handle the processing of this information (seems silly to me why these are boolean; checking to see if it will be succesful should happen before the function is called, IMO).

    I'm currently just testing the deposit function on the checking account, seeing if I get the correct total deposit balance at the end, and prints "Success!" for each succesful run.

    So far the code I've got is giving me an incorrect answer. I've stared at this for a very long time, and it usually takes a fresh look to spot the problem, so if you can, I'd appreciate any help. I should be getting around 200 as the answer, but it shows me "0." For some reason, the checking_balance is not being changed, even though I used 'float &balance' in my function :/

    Here is my code so far (please ignore the 'using namespace std;'...bugs me too, but the prof makes us use it):
    Code:
    #include <iostream>
    #include <iomanip>
    #include <fstream>
    #include <string>
    using namespace std;
    
    bool deposit(float,float);        // processes deposits
    bool withdrawal(float,float);     // processes withdrawals
    bool transfer(float,float,float); // processes transfers
    
    int main()
    { string user,filename;
      fstream transaction_file;
      float checking_balance,savings_balance;
      float amount;
      string account, action;
      
    // set both balances to $0.00
    
      checking_balance = 0.00;
      savings_balance = 0.00;
    
    // begin, prompt for name, and open the corresponding file
    
      cout << "Welcome to the Bank!\n\n"
           << "Please enter your name: ";
      cin >> user;
      
      filename = "/Users/Josh/Desktop/"+user+".txt";
      
      transaction_file.open(filename.data(),ios::in);
      
      while(!transaction_file.eof() && !transaction_file.fail())
        { transaction_file >> account,action,amount;
          if(account == "Checking" && action == "Deposit")
             if(deposit(checking_balance,amount))
                cout << "Success!" << endl;
         
        }
    
    
    
    // close the file
    
      transaction_file.close();
    
      cout << "Total amount of deposits = " << checking_balance << endl;
      
      
      return 0;
    }
    
    bool deposit(float &balance,float amount)
    { if(amount >= 1)
        { balance += amount;
          return (true);
        }
      else
        return (false);
    } 
      
        
     
  2. macrumors 68030

    superbovine

    Joined:
    Nov 7, 2003
    #2
    Just a guess.
    shouldn't your prototype be:

    Code:
    deposit(&float, float);
    
     
  3. macrumors 6502a

    Palad1

    Joined:
    Feb 24, 2004
    Location:
    London, UK
    #3
    Let's add some logging, shall we?
    Code:
    #define DEBUG 1
      while(!transaction_file.eof() && !transaction_file.fail())
        { 
        transaction_file >> account,action,amount;
    
    #ifdef DEBUG
        cerr<<"account :("<<account<< ") action : ("<<action<<") amount("<<amount<<")"<<endl;
    #endif 
    
          if(account == "Checking" && action == "Deposit")
          {
    #ifdef DEBUG
             cerr <<"Cheking deposit"<<endl;
    #endif
             if(deposit(checking_balance,amount))
             {
                cout << "Success!" << endl;
             }
          }
        }
    
    
    
    // close the file
    
      transaction_file.close();
    
      cout << "Total amount of deposits = " << checking_balance << endl;
      
      
      return 0;
    }
    
    bool deposit(float &balance,float amount)
    {
      if(amount >= 1.0F) // I think you may have had a casting problem as well
      {
    #if DEBUG
     cerr<<" deposit"<<endl;
    #endif
         balance += amount;
         return true;
      }
      else
      {
    #if DEBUG
     cerr<<" deposit <1 "<<endl;
    #endif
        return false;
      }
    }   
    
    I think your problem may lie in the parsing, you don't really check what you have, adding some logging will make your code easier to understand at runtime without using a debugger.

    Also, if( balance >=1) may pose a problem, as you are checking a float against an int, although the casting operator should be implicitly called, it never hurts to be correct : if (balance >= 1F)

    A final point: use curly braces, or don't, but don't mix, it makes code less readeable:
    Code:
    if(foo){
     dobar()
     woot(1);
    }else
      woot(-1);
    // vs
    if(foo)
    {
      dobar();
      woot(1);
    }
    else
    {
      woot(-1); 
    }
    
    Well, you may not like the way curly braces are distributed, so just choose your style, then BE CONSISTENT ;)

    Hope that helps,
    Palad1
     
  4. thread starter macrumors 68000

    Josh

    Joined:
    Mar 4, 2004
    Location:
    State College, PA
    #4
    Thanks for the help guys, I really appreciate it!

    (float &) was the problem. I also put the account actions/amounts/balances into arrays to make things go smoother.

    Thanks again :)
     
  5. macrumors 68030

    superbovine

    Joined:
    Nov 7, 2003
    #5
    for smaller projects, you can move function between your prototypes and int main(). Might make it easier to catch those errors. You could also just encapsulate them in .h file as well.
     
  6. thread starter macrumors 68000

    Josh

    Joined:
    Mar 4, 2004
    Location:
    State College, PA
    #6
    I will definitely keep that in mind. Glad I have MR to fall back on when I get stuck :eek:

    Although I'm very new at C++ (and programming in general), am I right in thinking that having the transaction functions in this project being boolean is illogical?

    The way were are supposed to use them in this project, is to compute the balance (either subtract from or add to it) givin the ammount and the balance, then return 'true' if that was possible (no amounts were negative, and the request was not larger than the balance). Otherwise, no values would be changed, and 'false' would be returned.

    Then, in the main program, if the function returned true, it would would display the details (type of transaction, amount, balance). If false, it would display "insufficient funds" or something similar.

    Would it not make more sense to have the function as void, and then just call the function, and let *it* decide which text to display? That not only saves repetition, but I cannot think of one reason why the other way would be better :confused:

    Here is a snippet to show you want I mean:
    Code:
    if(account[i]=="Checking" && action[i]=="Withdrawal")
            { cout << left << setw(25) << "Checking Withdrawal";
              if(withdrawal(checking_balance,amount[i]))
                { cout << right << setw(8) << amount[i]
                       << right << setw(13) << savings_balance
                       << " " << right << setw(13) << checking_balance << endl;
                }
              else
                { cout << right << setw(8) << amount[i] 
                       << right << setw(27) << "*Insufficient Funds*" << endl;
                }
    
    
    --- the function --
    
    bool withdrawal(float &balance,float amount)
    { if(amount >= 0.0 && amount <= balance)
        { balance -= amount;
          return (true);
        }
      else
        return (false);
    }
    
    
    Considering there are three types of functions (deposit, withdrawal, transfer) and two different accounts, the amount of code duplication is pretty high.

    I'd think something roughly like the example below would be more effecient:
    Code:
    if(account[i]=="Checking" && action[i]=="Withdrawal")
            { withdrawal(checking_balance,amount[i]);
    
           ....
            }
    
    
    ---- the function ---
    
    void withdrawal(float &balance,float amount)
    { if(amount >= 0.0 && amount <= balance)
        { balance -= amount;
          cout << right << setw(8) << amount[i]
                 << right << setw(13) << savings_balance
                 << " " << right << setw(13) << checking_balance << endl;
        }
      else
        cout << "Insufficient funds" << endl;
    }
    
    
     
  7. macrumors regular

    Joined:
    Apr 11, 2003
    #7
    i've had this same discussion with many people on many different occasions. 'there is more than one way to skin a cat' is what i hear pretty often. but, the most intelligent answer (at least as it seems to me) is that the main shouldn't have to worry about the business logic. the function should be a 'black box' so that you don't know how it does anything; it just sends back a true if what it did was successful. main shouldn't care about whether you can do it or not (or, again, what it is even doing) but only if it was a success. in a more complex project, you would probably design a class to handle all output, so putting that in this function wouldn't 'fit.'

    what if, in another part of a complex program, you didn't want the function to display anything? then you have to write a whole new set of functions (and yes, you could overload, but that's another conversation) that just do the math and no output. keeping everything 'loosely coupled' encourages reusability for you functions. each function should have only one, well, function! putting too much in a single function can cause reusability issues later on (when the projects get bigger).

    i hope someone else will join in on this topic and correct or verify my thoughts on this.
     
  8. thread starter macrumors 68000

    Josh

    Joined:
    Mar 4, 2004
    Location:
    State College, PA
    #8
    You do make a good point.

    But if this program for example was to not show output, the way we've been asked to write it would require removing the 6 blocks of identical output code.

    The other way I propose would involve removing only 2 indentical blocks of code (the transaction function calls upon deposit() and withdrawal() to move the money).

    Here is the code in its entirety so you can see the duplication that bugs me:
    Code:
    /* Program 10 by Josh Stevens 4/4/2006
    ** Asks user for a name, and then processes
    ** various bank transactions on the user's account
    ** based on a transaction file.
    *********************************************************/
    
    #include <iostream>
    #include <iomanip>
    #include <fstream>
    #include <string>
    using namespace std;
    
    bool deposit(float &,float);              // processes deposits
    bool withdrawal(float &,float);           // processes withdrawals
    bool transfer(float &,float &,float);     // processes transfers
    
    const int MAX = 50;                       // maximum number of expected transactions
    
    int main()
    { string user,filename;
      string account[MAX], action[MAX];
      fstream transaction_file;
      float amount[MAX];  // amount of the transaction
      float checking_balance,savings_balance;
      int i;  // counter
      int transactions; // the number of transactions taking place
      
    // set balances to $0.00, transactions to 0
    
      checking_balance = 0.00;
      savings_balance = 0.00;
      transactions = 0;
    
    // set transaction details to have no values
     
      for(i=0;i<MAX;i++)
        { amount[i] = 0.00;
          account[i] = "null";
          action[i] = "null";
        }
    
    // begin and prompt for name
    
      cout << "Welcome to the Bank!\n\n"
           << "Please enter your name: ";
      cin >> user;
      
      filename = "C:\\"+user+".txt";
      
    // open the file
    
      transaction_file.open(filename.data(),ios::in);
      
    // read the file into the arrays
    
      while(!transaction_file.eof() && !transaction_file.fail())
        { for(i=0;i<MAX;i++)
            { transaction_file >> account[i] >> action[i] >> amount[i];
            }    
        }
    
    // close the file
    
      transaction_file.close();
    
    // count number of transactions for future loops
    
      for(i=0;i<MAX && account[i] != "null";i++)
        transactions++;
    
    // set up display
    
      cout << "\nTransaction               Amount      Savings      Checking" << endl
           << "-------------------      --------    ---------    ----------" << endl;
    
    // process transactions and ouput the results
    
      cout << fixed << setprecision(2);
      
      for(i=0;i<transactions;i++)
    
    // checking deposit
    
        { if(account[i]=="Checking" && action[i]=="Deposit")
            { cout << left << setw(25) << "Checking Deposit";
              if(deposit(checking_balance,amount[i]))
                { cout << right << setw(8) << amount[i]
                       << right << setw(13) << savings_balance
                       << " " << right << setw(13) << checking_balance << endl;
                }
            }
    
    // savings deposit
    
          if(account[i]=="Savings" && action[i]=="Deposit")
            { cout << left << setw(25) << "Savings Deposit";
              if(deposit(savings_balance,amount[i]))
                { cout << right << setw(8) << amount[i]
                       << right << setw(13) << savings_balance
                       << " " << right << setw(13) << checking_balance << endl;
                }
            }
    
    // checking withdrawal
    
          if(account[i]=="Checking" && action[i]=="Withdrawal")
            { cout << left << setw(25) << "Checking Withdrawal";
              if(withdrawal(checking_balance,amount[i]))
                { cout << right << setw(8) << amount[i]
                       << right << setw(13) << savings_balance
                       << " " << right << setw(13) << checking_balance << endl;
                }
              else
                { cout << right << setw(8) << amount[i] 
                       << right << setw(27) << "*Insufficient Funds*" << endl;
                }
            }
    
    // savings withdrawal
    
          if(account[i]=="Savings" && action[i]=="Withdrawal")
            { cout << left << setw(25) << "Savings Withdrawal";
              if(withdrawal(savings_balance,amount[i]))
                { cout << right << setw(8) << amount[i]
                       << right << setw(13) << savings_balance
                       << " " << right << setw(13) << checking_balance << endl;
                }
              else
                { cout << right << setw(8) << amount[i] 
                       << right << setw(27) << "*Insufficient Funds*" << endl;
                }
            }
    
    // checking transfer (from checking to savings)
    
          if(account[i]=="Checking" && action[i]=="Transfer")
            { cout << left << setw(25) << "Checking Transfer";
              if(transfer(checking_balance,savings_balance,amount[i]))
                { cout << right << setw(8) << amount[i]
                       << right << setw(13) << savings_balance
                       << " " << right << setw(13) << checking_balance << endl;
                }
              else
                { cout << right << setw(8) << amount[i] 
                       << right << setw(27) << "*Insufficient Funds*" << endl;
                }
            }
    
    // savings transfer (from savings to checking)
    
          if(account[i]=="Savings" && action[i]=="Transfer")
            { cout << left << setw(25) << "Checking Transfer";
              if(transfer(savings_balance,checking_balance,amount[i]))
                { cout << right << setw(8) << amount[i]
                       << right << setw(13) << savings_balance
                       << " " << right << setw(13) << checking_balance << endl;
                }
              else
                { cout << right << setw(8) << amount[i] 
                       << right << setw(27) << "*Insufficient Funds*" << endl;
                }
            }
        }
    
    // thank user and quit
    
      cout << "\nThank you for visiting the bank, " << user << endl;
    
      return 0;
    }
    
    /* bool() takes source balance, destination balance, and the amount of the transaction.
    ** The amount is subtracted from the source and then added to the destination.
    ** Calls upon deposit() and withdrawal() to move the money.
    ***************************************************************************************/
    
    bool transfer(float &source,float &destination, float amount)
    { if(amount >= 0.0 && amount <= source)
        { withdrawal(source,amount);
          deposit(destination,amount);
          return (true);
        }
      else  
        return (false);
    }
    
    /* deposit() takes the account balance and transaction amount as parameters.
    ** If the amount is a positive number, it is added to the balance. 
    ** Otherwise, it does nothing and returns (false).
    ****************************************************************************/
    
    bool deposit(float &balance,float amount)
    { if(amount >= 0.0)
        { balance += amount;
          return (true);
        }
      else
        return (false);
    }
    
    /* withdrawal() takes the account balance and ammount as parameters.
    ** If the amount is positive, and less than the balance, the ammounts is
    ** subtracted from the balance. Otherwise, nothing happens and (false) is
    ** returned.
    ****************************************************************************/
    
    bool withdrawal(float &balance,float amount)
    { if(amount >= 0.0 && amount <= balance)
        { balance -= amount;
          return (true);
        }
      else
        return (false);
    }
    
     
  9. macrumors 6502a

    #9
    You are dumping the amount, saving's balance and checking balance for every error, regardless of cause. Suppose there are a hundred different account type and a hundred different transactions (and given the combinations of bank accounts, tax rates and interest rates that is not an outrageous proposition), are you going to dump all that information whenever there is an error?

    Instead, store the transaction type, balance and account type in variables along with the transaction result. Then write a single reporting function that will tell you what transaction went wrong, with which account and what the balance currently is.

    It is also trivial to redirect the output to a file or such like if you need to run 1000s of transactions and check them later.
     
  10. macrumors 65816

    MarkCollette

    Joined:
    Mar 6, 2003
    Location:
    Calgary, Canada
    #10
    Have you learned about classes yet? As a rule, if you're using parallel arrays, such as account, action, and amount, then you should make a class with fields for account, action, and amount, and have one single array of that class. Or better yet, since you're using STL string objects, you should make a vector of that class.

    Also, there are 2 accounts, and 3 actions, so instead of doing 6 if statements like:

    Code:
    if(account[i]=="Checking" && action[i]=="Deposit") {
    }
    else if(account[i]=="Savings" && action[i]=="Deposit") {
    }
    instead do:

    Code:
    if(action[i]=="Deposit") {
        if(account[i]=="Checking") {
            deposit(checking_balance,amount[i]);
        }
        else if(account[i]=="Savings") {
            deposit(savings_balance,amount[i]);
        }
    }
    Or better yet, when you parse in the account, you could test right away for the account type and instead of storing the string, store an integer constant, say 0 for Savings and 1 for Checking.

    Code:
    const int ACCOUNT_SAVINGS = 0;
    const int ACCOUNT_CHECKING = 1;
    const int NUM_ACCOUNTS = 2;
    ...
    int account[MAX];
    // Replaces checking_balance,savings_balance
    int balances[NUM_ACCOUNTS];
    ...
    if(action[i]=="Deposit") {
        deposit(balances[account[i]],amount[i]);
    }
    
    And the reason why transfer,withdrawl, and deposit are all bools, instead of checking first, is that bank transactions have to be atomic, meaning that the test for money has to happen right then and there, or else someone could use two different ATMs, and ask for their full balance at both, and get twice the money they should be able to.

    Also, you should test your inputs right when you read them in. So, if the file says "Checkfgjsdhg" instead of "Checking", you should fail that input file before proceeding, unlike right now, where it's right when you do the processing of calling transfer,deposit or withdraw.

    Another reason for checking input data right away, is so you can decouple the parsed file representation from the internally stored representation. Remember how I showed that storing the account as an integer instead of a string made things easier? Well that decoupling of input format and internal format allows that. Also, what happens when your code has to input data from multiple sources? Like from a web page, or an XML file, or a database.
     
  11. thread starter macrumors 68000

    Josh

    Joined:
    Mar 4, 2004
    Location:
    State College, PA
    #11
    ^^ thanks a lot - lot of great info there that definitely makes more sense.

    We *just* learned about classes yesterday evening, right after needing to hand in that assignment. :eek:
     
  12. macrumors 65816

    MarkCollette

    Joined:
    Mar 6, 2003
    Location:
    Calgary, Canada
    #12
    No problem. It's quite typical to give an assignment that would benefit from classes before they teach classes, to make it all the more apparent how useful classes are. Same thing where you get an assignment doing math with 5 variables, and then learn about arrays right afterwards, etc. :)
     
  13. macrumors 68030

    superbovine

    Joined:
    Nov 7, 2003
    #13
    just out of curiosity what compiler you guys using to grade the assignments.
     

Share This Page