Welcome to Dream.In.Code
Getting C++ Help is Easy!

Join 136,173 C++ Programmers for FREE! Get instant access to thousands of C++ experts, tutorials, code snippets, and more! There are 1,947 people online right now. Registration is fast and FREE... Join Now!




Review and Critique

 
Reply to this topicStart new topic

Review and Critique, Bank I/O program

cooplis
5 Dec, 2007 - 06:03 AM
Post #1

D.I.C Head
**

Joined: 11 Sep, 2007
Posts: 77


My Contributions
This is almost finish, I have a glitch it does not output the info from the file. Please critique.

CODE

#include <fstream>
#include <iostream>
#include <iomanip>
#include <cstdlib>
#include <stdio.h>

int Transcode;    
double bal, balance, Begbal, transamount;

const float            ATMFEE    = 2.00,
                    SVCFEE    = 10.00;            


void DisplayTitle();  

double DispalyBal(double);
double PROCESSCHECK    (double, double);
double PROCESSDEP    (double, double, double);
double PROCESSATM    (double, double, double);  
double AUTOPAY      (double, double, double);
void Getdata(int, double);
//double Getbegbal(ifstream& in_stream);
struct transRec
{
int Transcode;
double transamount;
};

using namespace std;




int  main()
{
ifstream in_stream;
ofstream out_stream;    
        
    DisplayTitle();
    cout<< endl;
    
    cout.setf(ios::fixed);
    cout.setf(ios::showpoint);
    cout.precision(2);      
    
    ifstream checkin;
    in_stream.open("e:\\checkin.txt");
    out_stream.open("e:\\checkout.txt");
    
     if (checkin.is_open()) {
          cout << "File is open" << endl;
     }
    
     Getdata(transRec);
    
    else {
          cout << "File is closed" << endl;
     }
    system("Pause");
    
      
      
switch (Transcode)
   {
     case 1:  balance = PROCESSCHECK(bal, transamount);    
               break;
     case 2:   balance = PROCESSDEP(bal, transamount, balance);
               break;
     case 3:   balance = PROCESSATM(bal, transamount, balance);
               break;  
     case 4:  balance = AUTOPAY(transamount, bal, balance);
      break;  
}

system("pause");
}

void DisplayTitle()
{
    cout << "            Check Register      \n\n" <<endl;
    cout <<             "    1. Check      \n";
    cout <<             "    2. Deposit    \n ";
    cout <<             "   3. ATM (Withdrawal Only) \n";
    cout <<             "    4. Autopay              \n\n";
}

/*void DisplayBal()
{    
    return balance;
}
*/
double Getbegbal(ifstream& in_stream)
{
    in_stream.open("e:\\checkin.txt");
    
}

void Getdata(int Transcode, double transamount, transRec)
{
    cout <<  "    Enter transaction code (0 to exit)     ";
    cin >> Transcode;
    cout <<endl;
}

double PROCESSCHECK(double bal, double transamount)
{
         balance = (bal - transamount);
         cout << "Check = " << transamount << setw(20)
         << "Balance = " << setw(6) << balance;
         cout <<"\n";
         return balance;
}      
        

double PROCESSDEP(double bal, double balance, double transamount)
{
    
      cout << "Deposit Amount=" <<transamount;    
      cout << "Service Charge= " << SVCFEE <<endl;
      cout << setw(16) << "Balance = " << balance<<endl;
        balance = (transamount+bal)-SVCFEE;
        return balance;
}    

double PROCESSATM(double bal, double transamount, double balance)
{
   balance = bal-transamount;    
   cout << setw(16)<<"ATM Amount=" <<transamount<<endl;
   cout << setw(16)<<"ATM Fee   =$" <<ATMFEE <<endl;
   cout << setw(16) <<"Balance = "  << (balance-ATMFEE) <<endl;
  return balance;
}

double AUTOPAY(double transamount, double balance, double bal)
{
    cout << "Withdrawal Amount=" << transamount;
    cout << balance;
    balance = bal-transamount;
    return balance;    
}

User is offlineProfile CardPM
+Quote Post

lockdown
RE: Review And Critique
5 Dec, 2007 - 06:10 AM
Post #2

D.I.C Regular
Group Icon

Joined: 29 Sep, 2007
Posts: 376



Thanked: 1 times
Expert In: PC, Support

My Contributions
I see you add to your code I helped you with. Very nice. I will try and find the error smile.gif
User is offlineProfile CardPM
+Quote Post

cooplis
RE: Review And Critique
5 Dec, 2007 - 08:58 AM
Post #3

D.I.C Head
**

Joined: 11 Sep, 2007
Posts: 77


My Contributions
QUOTE(lockdown @ 5 Dec, 2007 - 07:10 AM) *

I see you add to your code I helped you with. Very nice. I will try and find the error smile.gif



Thanks, I still have a few more things to add to make this project complete.

Cooplis
User is offlineProfile CardPM
+Quote Post

NickDMax
RE: Review And Critique
5 Dec, 2007 - 10:08 AM
Post #4

2B||!2B
Group Icon

Joined: 18 Feb, 2007
Posts: 2,858



Thanked: 49 times
Dream Kudos: 550
My Contributions
well you DID say critique... So hold on (not trying to be mean).

First off lets talk about global variables: They really should not be used. Like many style "rule" this is not really a rule some much as a lesson. Over the years programmers have learned over and over that global variables can introduce wickedly hard to track down bugs. Especially when linked together with some of the other stylistic things I will mention.

Using the same name for different variables: You have used the variable name 'Transcode' in a number of places. First off it is in the global scope so if you are not carful with scoping you may change the variable in a local scope and have unintended consequences on the global variable (see the note above). Secondly it is inside a struct which is actually ok-ish since as a member of the struct it gets prefixed with the struct's variable name. Then you use it as an argument to Getdata()... but it is not passed by reference, and that is not the global... is that a bug I see already?

You use function names that are all caps: This is normally reserved for macro's. Over the years programmers found that macro's can be cause problems if the programmer does not keep in mind that they are macro's, to help mitigate this situation the convention is that macro's are named with all caps so that other programmers can instantly recognize them.

The style points above do not really make a huge difference, but they can make your life much more livable when there are problems.
User is offlineProfile CardPM
+Quote Post

cooplis
RE: Review And Critique
5 Dec, 2007 - 10:21 AM
Post #5

D.I.C Head
**

Joined: 11 Sep, 2007
Posts: 77


My Contributions
QUOTE(NickDMax @ 5 Dec, 2007 - 11:08 AM) *

well you DID say critique... So hold on (not trying to be mean).

First off lets talk about global variables: They really should not be used. Like many style "rule" this is not really a rule some much as a lesson. Over the years programmers have learned over and over that global variables can introduce wickedly hard to track down bugs. Especially when linked together with some of the other stylistic things I will mention.

Using the same name for different variables: You have used the variable name 'Transcode' in a number of places. First off it is in the global scope so if you are not carful with scoping you may change the variable in a local scope and have unintended consequences on the global variable (see the note above). Secondly it is inside a struct which is actually ok-ish since as a member of the struct it gets prefixed with the struct's variable name. Then you use it as an argument to Getdata()... but it is not passed by reference, and that is not the global... is that a bug I see already?

You use function names that are all caps: This is normally reserved for macro's. Over the years programmers found that macro's can be cause problems if the programmer does not keep in mind that they are macro's, to help mitigate this situation the convention is that macro's are named with all caps so that other programmers can instantly recognize them.

The style points above do not really make a huge difference, but they can make your life much more livable when there are problems.

So your saying it's a mess. I know because when I compile it kept bringing up errors then I would move a function or something and it would run but not how I planned for it to.

Thank you, I'll keep working on it.
User is offlineProfile CardPM
+Quote Post

cooplis
RE: Review And Critique
6 Dec, 2007 - 05:40 AM
Post #6

D.I.C Head
**

Joined: 11 Sep, 2007
Posts: 77


My Contributions
Any other suggestions as to how to make this thing actually give me the output. If I do outstream and cout will that cause me any problems?
User is offlineProfile CardPM
+Quote Post

baavgai
RE: Review And Critique
6 Dec, 2007 - 06:08 AM
Post #7

Dreaming Coder
Group Icon

Joined: 16 Oct, 2007
Posts: 2,019



Thanked: 105 times
Dream Kudos: 475
Expert In: C, C++, Java, C#, ASP.NET, PHP, Perl, Python, Oracle, SQL Server, MySql, HTML, JavaScript, Lua

My Contributions
QUOTE(cooplis @ 6 Dec, 2007 - 08:40 AM) *

Any other suggestions as to how to make this thing actually give me the output. If I do outstream and cout will that cause me any problems?


I entirely agree on the global thing, ouch. Also, bal and balance, different how?

Minor point:
CODE

balance = PROCESSCHECK(bal, transamount);    
balance = PROCESSDEP(bal, transamount, balance);
balance = PROCESSATM(bal, transamount, balance);
balance = AUTOPAY(transamount, bal, balance);


Hmm, I'd make this change, to be safe:
CODE

balance = AUTOPAY(bal, transamount, balance);


Which leads to a final note, objects? This code is begging for a class or two. You have three functions that process identical parameters, with the fourth differing in only a minor way. Any way to bring all that data together in a unified manner?


User is offlineProfile CardPM
+Quote Post

cooplis
RE: Review And Critique
6 Dec, 2007 - 06:37 AM
Post #8

D.I.C Head
**

Joined: 11 Sep, 2007
Posts: 77


My Contributions
QUOTE(baavgai @ 6 Dec, 2007 - 07:08 AM) *

QUOTE(cooplis @ 6 Dec, 2007 - 08:40 AM) *

Any other suggestions as to how to make this thing actually give me the output. If I do outstream and cout will that cause me any problems?


I entirely agree on the global thing, ouch. Also, bal and balance, different how?

Minor point:
CODE

balance = PROCESSCHECK(bal, transamount);    
balance = PROCESSDEP(bal, transamount, balance);
balance = PROCESSATM(bal, transamount, balance);
balance = AUTOPAY(transamount, bal, balance);


Hmm, I'd make this change, to be safe:
CODE

balance = AUTOPAY(bal, transamount, balance);


Which leads to a final note, objects? This code is begging for a class or two. You have three functions that process identical parameters, with the fourth differing in only a minor way. Any way to bring all that data together in a unified manner?

That's my question what can I do to make this program less combursome. Take something out or combine some functions. And I still have problems just opening my file. I get so confused but I want to understand. I am retaking this class next semester but I want to go in with a little knowledge of what I am doing. Help!
User is offlineProfile CardPM
+Quote Post

cooplis
RE: Review And Critique
6 Dec, 2007 - 12:50 PM
Post #9

D.I.C Head
**

Joined: 11 Sep, 2007
Posts: 77


My Contributions
lockdown, NickDMax, and baavgai, any other suggestions?
User is offlineProfile CardPM
+Quote Post

baavgai
RE: Review And Critique
6 Dec, 2007 - 04:37 PM
Post #10

Dreaming Coder
Group Icon

Joined: 16 Oct, 2007
Posts: 2,019



Thanked: 105 times
Dream Kudos: 475
Expert In: C, C++, Java, C#, ASP.NET, PHP, Perl, Python, Oracle, SQL Server, MySql, HTML, JavaScript, Lua

My Contributions
QUOTE(cooplis @ 6 Dec, 2007 - 03:50 PM) *
any other suggestions?


I really seems you're looking to deal with some kind of checkbook? Just maintaining a single transaction does not seem to be the way to go. Without creating any custom objects, but killing the globals, here's one way you could do it:

CODE
#include <stdio.h>
#include <fstream>
#include <iostream>
#include <iomanip>
#include <vector>

using namespace std;

const double ATMFEE = -2.00;
const double SVCFEE = -10.00;            

enum TransCodeType { OpeningBalance, Check, Deposit, Atm, AtmFee, ServiceFee, Withdrawl };

typedef struct TransType {
   TransCodeType transCode;
   double amount;
   double balance;
} TransType;

typedef std::vector<TransType> LedgerType;

void Print(const TransCodeType transCode) {
   string s = "";
   switch(transCode) {
      case OpeningBalance: s = "Opening"; break;
      case Check: s = "Check"; break;
      case Deposit: s = "Deposit"; break;
      case Atm: s = "Atm"; break;
      case AtmFee: s = "Atm Fee"; break;
      case ServiceFee: s = "Service Fee"; break;
      case Withdrawl: s = "Withdrawl"; break;
   }
   cout << left << setw(20) << s;
}

void Print(const TransType &transaction) {
   Print(transaction.transCode);
   cout << right << fixed << setprecision(2);
   cout << setw(12) << transaction.amount << setw(12) << transaction.balance << endl;
}

void Print(const LedgerType &ledger) {
   int i=0;
   for(vector<TransType>::const_iterator it = ledger.begin(); it!=ledger.end(); it++) {
      cout << right << fixed << setprecision(0) << setw(5) << ++i << ")  ";
      Print(*it);
   }
}


double GetBalance(LedgerType &ledger) {
   int index = ledger.size();
   if (index==0) {
      return 0;
   } else {
      return ledger.at(index - 1).balance;
   }
}

void AddTransaction(LedgerType &ledger, TransCodeType transCode, const double amount) {
   TransType trans;
   trans.transCode = transCode;
   trans.amount = amount;
   trans.balance = trans.amount + GetBalance(ledger);
   ledger.push_back(trans);
}

void ProcessOpeningBalance(LedgerType &ledger, const double amount) {
   ledger.clear();
   AddTransaction(ledger, OpeningBalance, amount);
}      

void ProcessCheck(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Check, -amount);
}      

void ProcessDeposit(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Deposit, amount);
   AddTransaction(ledger, ServiceFee, SVCFEE);
}      

void ProcessAtm(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Atm, -amount);
   AddTransaction(ledger, AtmFee, ATMFEE);
}      

void ProcessWithdrawl(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Withdrawl, -amount);
}      


int  main() {
   LedgerType ledger;
   ProcessOpeningBalance(ledger, 1000);
   ProcessDeposit(ledger, 250);
   ProcessCheck(ledger, 125);
   ProcessAtm(ledger, 100);
   ProcessWithdrawl(ledger, 20);
   Print(ledger);
}


Hope this helps.

User is offlineProfile CardPM
+Quote Post

cooplis
RE: Review And Critique
7 Dec, 2007 - 04:43 AM
Post #11

D.I.C Head
**

Joined: 11 Sep, 2007
Posts: 77


My Contributions
QUOTE(baavgai @ 6 Dec, 2007 - 05:37 PM) *

QUOTE(cooplis @ 6 Dec, 2007 - 03:50 PM) *
any other suggestions?


I really seems you're looking to deal with some kind of checkbook? Just maintaining a single transaction does not seem to be the way to go. Without creating any custom objects, but killing the globals, here's one way you could do it:

CODE
#include <stdio.h>
#include <fstream>
#include <iostream>
#include <iomanip>
#include <vector>

using namespace std;

const double ATMFEE = -2.00;
const double SVCFEE = -10.00;            

enum TransCodeType { OpeningBalance, Check, Deposit, Atm, AtmFee, ServiceFee, Withdrawl };

typedef struct TransType {
   TransCodeType transCode;
   double amount;
   double balance;
} TransType;

typedef std::vector<TransType> LedgerType;

void Print(const TransCodeType transCode) {
   string s = "";
   switch(transCode) {
      case OpeningBalance: s = "Opening"; break;
      case Check: s = "Check"; break;
      case Deposit: s = "Deposit"; break;
      case Atm: s = "Atm"; break;
      case AtmFee: s = "Atm Fee"; break;
      case ServiceFee: s = "Service Fee"; break;
      case Withdrawl: s = "Withdrawl"; break;
   }
   cout << left << setw(20) << s;
}

void Print(const TransType &transaction) {
   Print(transaction.transCode);
   cout << right << fixed << setprecision(2);
   cout << setw(12) << transaction.amount << setw(12) << transaction.balance << endl;
}

void Print(const LedgerType &ledger) {
   int i=0;
   for(vector<TransType>::const_iterator it = ledger.begin(); it!=ledger.end(); it++) {
      cout << right << fixed << setprecision(0) << setw(5) << ++i << ")  ";
      Print(*it);
   }
}


double GetBalance(LedgerType &ledger) {
   int index = ledger.size();
   if (index==0) {
      return 0;
   } else {
      return ledger.at(index - 1).balance;
   }
}

void AddTransaction(LedgerType &ledger, TransCodeType transCode, const double amount) {
   TransType trans;
   trans.transCode = transCode;
   trans.amount = amount;
   trans.balance = trans.amount + GetBalance(ledger);
   ledger.push_back(trans);
}

void ProcessOpeningBalance(LedgerType &ledger, const double amount) {
   ledger.clear();
   AddTransaction(ledger, OpeningBalance, amount);
}      

void ProcessCheck(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Check, -amount);
}      

void ProcessDeposit(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Deposit, amount);
   AddTransaction(ledger, ServiceFee, SVCFEE);
}      

void ProcessAtm(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Atm, -amount);
   AddTransaction(ledger, AtmFee, ATMFEE);
}      

void ProcessWithdrawl(LedgerType &ledger, const double amount) {
   AddTransaction(ledger, Withdrawl, -amount);
}      


int  main() {
   LedgerType ledger;
   ProcessOpeningBalance(ledger, 1000);
   ProcessDeposit(ledger, 250);
   ProcessCheck(ledger, 125);
   ProcessAtm(ledger, 100);
   ProcessWithdrawl(ledger, 20);
   Print(ledger);
}


Hope this helps.

Thanks your good, but yes it was like an ATM machine. She wanted us to pull it from a file.

Thanks again.
User is offlineProfile CardPM
+Quote Post

baavgai
RE: Review And Critique
7 Dec, 2007 - 07:18 AM
Post #12

Dreaming Coder
Group Icon

Joined: 16 Oct, 2007
Posts: 2,019



Thanked: 105 times
Dream Kudos: 475
Expert In: C, C++, Java, C#, ASP.NET, PHP, Perl, Python, Oracle, SQL Server, MySql, HTML, JavaScript, Lua

My Contributions
Reading from a file should not be a big challenge. However, first you really need to know the format of the file.

You would start out with an empty list and simply have the file reader add elements to the list.
User is offlineProfile CardPM
+Quote Post

Reply to this topicStart new topic
Time is now: 12/2/08 12:46AM

Live C++ Help!

C++ Tutorials

Reference Sheets

C++ Snippets

DIC Chatroom

Bye Bye Ads

Monthly Drawing

Thumb Drive

Top Contributors

Top 10 Kudos This Month