MonthlyBudget tracker that tracks user's budget and compares it to their expenses

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
6
down vote

favorite












The program will track the monthly budget of the user and will then compare it to the expenses of the user. The data members of the structs will be sent to binary files. Once the results are displayed to the user, the results will be sent to a text file. Also be advised, My teacher is very old schooled so I apologize if some of the methods I've used are out of date.



#include <cstdlib>
#include <limits>
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>
#include <cmath>
#include <cctype>
using namespace std;

struct MonthlyBudget

double housing;
double utilites;
double houseHold;
double transportation;
double food;
double medical;
double insurance;
double entertainment;
double clothinng;
double misc;
;

struct MonthlyExpenses

double housingEx;
double utilitesEx;
double householdEx;
double transportationEx;
double foodEx;
double medicalEx;
double insuranceEx;
double entertainmentEx;
double clothinngEx;
double miscEx;
;

//prototypes declared outside of main so main function logic isn't distorted
void describeProgram();
void getMonths(int& months);
void getMonthlyBudget(fstream& budgetFile, int months);
void getMonthlyExpenses(fstream& expenseFile, int months);
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months);

/**
*
* @return 0
*/
int main()

int months = 1;
fstream budgetFile, expenseFile;

describeProgram();
getMonths(months);
getMonthlyBudget(budgetFile, months);
getMonthlyExpenses(expenseFile, months);
getMonthlyReport(budgetFile, expenseFile, months);

return 0;


/**
*
*/
void describeProgram()

cout<<
"This program will ask you for your desired budget for housing, "
"utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
" of money you spent on such expenses. n First, the program will ask you for how "
"many months you would like analyzed. These numbers entered will be put into "
"a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
"It will then calculate the difference and display the budget, n the expenses, "
"and over/under in a tabular format.n";


/**
*
* @param months
*/
void getMonths(int& months)

cout<<"How many months would you like to have analyzed? ";
cin>>months;

while(cin.fail())

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(),'n');
cout << "Enter valid input ";
cin >> months;




/**
*
* @param budgetFile
* @param months
*/
void getMonthlyBudget(fstream& budgetFile, int months)
ios::binary);
if(budgetFile.fail())

cout<<"Could not find file: budget.bin n";
system("read"); //act as system("PAUSE")


try

do

cout<<"Enter your housing budget for month "<<count<<":n";
cin>>mb.housing;
cout<<"Enter your utilities budget for month "<<count<<":n";
cin>>mb.utilites;
cout<<"Enter your household expense budget for month "<<count<<":n";
cin>>mb.houseHold;
cout<<"Enter your transportation budget for month "<<count<<":n";
cin>>mb.transportation;
cout<<"Enter your food budget for month "<<count<<":n";
cin>>mb.food;
cout<<"Enter your medical budget for month "<<count<<":n";
cin>>mb.medical;
cout<<"Enter your insurance budget for month "<<count<<":n";
cin>>mb.insurance;
cout<<"Enter your entertainment budget for month "<<count<<":n";
cin>>mb.entertainment;
cout<<"Enter your clothing budget for month "<<count<<":n";
cin>>mb.clothinng;
cout<<"Enter your Miscellaneous budget for month "<<count<<":n";
cin>>mb.misc;

budgetFile.write(reinterpret_cast<char*>(&mb),sizeof(mb));
count++;
months --;
while(months != 0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";


budgetFile.close();


/**
*
* @param expenseFile
* @param months
*/
void getMonthlyExpenses(fstream& expenseFile, int months)
ios::binary);
if(expenseFile.fail())

cout<<"Could not open expenses.bin n";
system("read");

try

do

cout<<"Enter your housing expenses for month "<<count<<":n";
cin>>me.housingEx;
cout<<"Enter your utilities expenses for month "<<count<<":n";
cin>>me.utilitesEx;
cout<<"Enter the cost of your house hold expenses for month "<<count<<":n";
cin>>me.householdEx;
cout<<"Enter your transportation expenses for month "<<count<<":n";
cin>>me.transportationEx;
cout<<"Enter your food expenses for month "<<count<<":n";
cin>>me.foodEx;
cout<<"Enter your medical expenses for month "<<count<<":n";
cin>>me.medicalEx;
cout<<"Enter your insurance expenses for month "<<count<<":n";
cin>>me.insuranceEx;
cout<<"Enter your entertainment expenses for month "<<count<<":n";
cin>>me.entertainmentEx;
cout<<"Enter your clothing expenses for month "<<count<<":n";
cin>>me.clothinngEx;
cout<<"Enter your Miscellaneous expenses for month "<<count<<":n";
cin>>me.miscEx;

expenseFile.write(reinterpret_cast<char*>(&me),sizeof(me));
count++;
months--;
while(months!=0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";

expenseFile.close();


/**
*
* @param budgetFile
* @param expenseFile
* @param months
*/
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

MonthlyBudget mb;
MonthlyExpenses me;
ofstream toFile;

budgetFile.open("budget.bin", ios::in






share|improve this question



















  • As I am not any expert in cpp, I prefer to post short advices instead of full answer: 1) please make a spot that budget and expenses are the same data structures in essence but only have different purpose. 2) instead of using fixed structs of doubles you can use dictionary-like data structure and iterate over its keys. 3) reading budget and expenses for month from the user is in essence the same but under different name 'budget'/'expenses' and into different target variable.
    – mpasko256
    Mar 1 at 13:31










  • Does your teacher enforce the rather curious use of system("read")?
    – yuri
    Mar 1 at 15:11










  • no, he does however encourage system("PAUSE") which I heard was worse. I just used read because I saw a few forums encouraging that instead. What would be the best option?
    – Zachariah
    Mar 1 at 16:37










  • @Zachariah I don't know about the best but cin.get() seems like a better choice.
    – yuri
    Mar 1 at 16:52

















up vote
6
down vote

favorite












The program will track the monthly budget of the user and will then compare it to the expenses of the user. The data members of the structs will be sent to binary files. Once the results are displayed to the user, the results will be sent to a text file. Also be advised, My teacher is very old schooled so I apologize if some of the methods I've used are out of date.



#include <cstdlib>
#include <limits>
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>
#include <cmath>
#include <cctype>
using namespace std;

struct MonthlyBudget

double housing;
double utilites;
double houseHold;
double transportation;
double food;
double medical;
double insurance;
double entertainment;
double clothinng;
double misc;
;

struct MonthlyExpenses

double housingEx;
double utilitesEx;
double householdEx;
double transportationEx;
double foodEx;
double medicalEx;
double insuranceEx;
double entertainmentEx;
double clothinngEx;
double miscEx;
;

//prototypes declared outside of main so main function logic isn't distorted
void describeProgram();
void getMonths(int& months);
void getMonthlyBudget(fstream& budgetFile, int months);
void getMonthlyExpenses(fstream& expenseFile, int months);
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months);

/**
*
* @return 0
*/
int main()

int months = 1;
fstream budgetFile, expenseFile;

describeProgram();
getMonths(months);
getMonthlyBudget(budgetFile, months);
getMonthlyExpenses(expenseFile, months);
getMonthlyReport(budgetFile, expenseFile, months);

return 0;


/**
*
*/
void describeProgram()

cout<<
"This program will ask you for your desired budget for housing, "
"utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
" of money you spent on such expenses. n First, the program will ask you for how "
"many months you would like analyzed. These numbers entered will be put into "
"a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
"It will then calculate the difference and display the budget, n the expenses, "
"and over/under in a tabular format.n";


/**
*
* @param months
*/
void getMonths(int& months)

cout<<"How many months would you like to have analyzed? ";
cin>>months;

while(cin.fail())

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(),'n');
cout << "Enter valid input ";
cin >> months;




/**
*
* @param budgetFile
* @param months
*/
void getMonthlyBudget(fstream& budgetFile, int months)
ios::binary);
if(budgetFile.fail())

cout<<"Could not find file: budget.bin n";
system("read"); //act as system("PAUSE")


try

do

cout<<"Enter your housing budget for month "<<count<<":n";
cin>>mb.housing;
cout<<"Enter your utilities budget for month "<<count<<":n";
cin>>mb.utilites;
cout<<"Enter your household expense budget for month "<<count<<":n";
cin>>mb.houseHold;
cout<<"Enter your transportation budget for month "<<count<<":n";
cin>>mb.transportation;
cout<<"Enter your food budget for month "<<count<<":n";
cin>>mb.food;
cout<<"Enter your medical budget for month "<<count<<":n";
cin>>mb.medical;
cout<<"Enter your insurance budget for month "<<count<<":n";
cin>>mb.insurance;
cout<<"Enter your entertainment budget for month "<<count<<":n";
cin>>mb.entertainment;
cout<<"Enter your clothing budget for month "<<count<<":n";
cin>>mb.clothinng;
cout<<"Enter your Miscellaneous budget for month "<<count<<":n";
cin>>mb.misc;

budgetFile.write(reinterpret_cast<char*>(&mb),sizeof(mb));
count++;
months --;
while(months != 0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";


budgetFile.close();


/**
*
* @param expenseFile
* @param months
*/
void getMonthlyExpenses(fstream& expenseFile, int months)
ios::binary);
if(expenseFile.fail())

cout<<"Could not open expenses.bin n";
system("read");

try

do

cout<<"Enter your housing expenses for month "<<count<<":n";
cin>>me.housingEx;
cout<<"Enter your utilities expenses for month "<<count<<":n";
cin>>me.utilitesEx;
cout<<"Enter the cost of your house hold expenses for month "<<count<<":n";
cin>>me.householdEx;
cout<<"Enter your transportation expenses for month "<<count<<":n";
cin>>me.transportationEx;
cout<<"Enter your food expenses for month "<<count<<":n";
cin>>me.foodEx;
cout<<"Enter your medical expenses for month "<<count<<":n";
cin>>me.medicalEx;
cout<<"Enter your insurance expenses for month "<<count<<":n";
cin>>me.insuranceEx;
cout<<"Enter your entertainment expenses for month "<<count<<":n";
cin>>me.entertainmentEx;
cout<<"Enter your clothing expenses for month "<<count<<":n";
cin>>me.clothinngEx;
cout<<"Enter your Miscellaneous expenses for month "<<count<<":n";
cin>>me.miscEx;

expenseFile.write(reinterpret_cast<char*>(&me),sizeof(me));
count++;
months--;
while(months!=0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";

expenseFile.close();


/**
*
* @param budgetFile
* @param expenseFile
* @param months
*/
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

MonthlyBudget mb;
MonthlyExpenses me;
ofstream toFile;

budgetFile.open("budget.bin", ios::in






share|improve this question



















  • As I am not any expert in cpp, I prefer to post short advices instead of full answer: 1) please make a spot that budget and expenses are the same data structures in essence but only have different purpose. 2) instead of using fixed structs of doubles you can use dictionary-like data structure and iterate over its keys. 3) reading budget and expenses for month from the user is in essence the same but under different name 'budget'/'expenses' and into different target variable.
    – mpasko256
    Mar 1 at 13:31










  • Does your teacher enforce the rather curious use of system("read")?
    – yuri
    Mar 1 at 15:11










  • no, he does however encourage system("PAUSE") which I heard was worse. I just used read because I saw a few forums encouraging that instead. What would be the best option?
    – Zachariah
    Mar 1 at 16:37










  • @Zachariah I don't know about the best but cin.get() seems like a better choice.
    – yuri
    Mar 1 at 16:52













up vote
6
down vote

favorite









up vote
6
down vote

favorite











The program will track the monthly budget of the user and will then compare it to the expenses of the user. The data members of the structs will be sent to binary files. Once the results are displayed to the user, the results will be sent to a text file. Also be advised, My teacher is very old schooled so I apologize if some of the methods I've used are out of date.



#include <cstdlib>
#include <limits>
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>
#include <cmath>
#include <cctype>
using namespace std;

struct MonthlyBudget

double housing;
double utilites;
double houseHold;
double transportation;
double food;
double medical;
double insurance;
double entertainment;
double clothinng;
double misc;
;

struct MonthlyExpenses

double housingEx;
double utilitesEx;
double householdEx;
double transportationEx;
double foodEx;
double medicalEx;
double insuranceEx;
double entertainmentEx;
double clothinngEx;
double miscEx;
;

//prototypes declared outside of main so main function logic isn't distorted
void describeProgram();
void getMonths(int& months);
void getMonthlyBudget(fstream& budgetFile, int months);
void getMonthlyExpenses(fstream& expenseFile, int months);
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months);

/**
*
* @return 0
*/
int main()

int months = 1;
fstream budgetFile, expenseFile;

describeProgram();
getMonths(months);
getMonthlyBudget(budgetFile, months);
getMonthlyExpenses(expenseFile, months);
getMonthlyReport(budgetFile, expenseFile, months);

return 0;


/**
*
*/
void describeProgram()

cout<<
"This program will ask you for your desired budget for housing, "
"utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
" of money you spent on such expenses. n First, the program will ask you for how "
"many months you would like analyzed. These numbers entered will be put into "
"a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
"It will then calculate the difference and display the budget, n the expenses, "
"and over/under in a tabular format.n";


/**
*
* @param months
*/
void getMonths(int& months)

cout<<"How many months would you like to have analyzed? ";
cin>>months;

while(cin.fail())

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(),'n');
cout << "Enter valid input ";
cin >> months;




/**
*
* @param budgetFile
* @param months
*/
void getMonthlyBudget(fstream& budgetFile, int months)
ios::binary);
if(budgetFile.fail())

cout<<"Could not find file: budget.bin n";
system("read"); //act as system("PAUSE")


try

do

cout<<"Enter your housing budget for month "<<count<<":n";
cin>>mb.housing;
cout<<"Enter your utilities budget for month "<<count<<":n";
cin>>mb.utilites;
cout<<"Enter your household expense budget for month "<<count<<":n";
cin>>mb.houseHold;
cout<<"Enter your transportation budget for month "<<count<<":n";
cin>>mb.transportation;
cout<<"Enter your food budget for month "<<count<<":n";
cin>>mb.food;
cout<<"Enter your medical budget for month "<<count<<":n";
cin>>mb.medical;
cout<<"Enter your insurance budget for month "<<count<<":n";
cin>>mb.insurance;
cout<<"Enter your entertainment budget for month "<<count<<":n";
cin>>mb.entertainment;
cout<<"Enter your clothing budget for month "<<count<<":n";
cin>>mb.clothinng;
cout<<"Enter your Miscellaneous budget for month "<<count<<":n";
cin>>mb.misc;

budgetFile.write(reinterpret_cast<char*>(&mb),sizeof(mb));
count++;
months --;
while(months != 0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";


budgetFile.close();


/**
*
* @param expenseFile
* @param months
*/
void getMonthlyExpenses(fstream& expenseFile, int months)
ios::binary);
if(expenseFile.fail())

cout<<"Could not open expenses.bin n";
system("read");

try

do

cout<<"Enter your housing expenses for month "<<count<<":n";
cin>>me.housingEx;
cout<<"Enter your utilities expenses for month "<<count<<":n";
cin>>me.utilitesEx;
cout<<"Enter the cost of your house hold expenses for month "<<count<<":n";
cin>>me.householdEx;
cout<<"Enter your transportation expenses for month "<<count<<":n";
cin>>me.transportationEx;
cout<<"Enter your food expenses for month "<<count<<":n";
cin>>me.foodEx;
cout<<"Enter your medical expenses for month "<<count<<":n";
cin>>me.medicalEx;
cout<<"Enter your insurance expenses for month "<<count<<":n";
cin>>me.insuranceEx;
cout<<"Enter your entertainment expenses for month "<<count<<":n";
cin>>me.entertainmentEx;
cout<<"Enter your clothing expenses for month "<<count<<":n";
cin>>me.clothinngEx;
cout<<"Enter your Miscellaneous expenses for month "<<count<<":n";
cin>>me.miscEx;

expenseFile.write(reinterpret_cast<char*>(&me),sizeof(me));
count++;
months--;
while(months!=0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";

expenseFile.close();


/**
*
* @param budgetFile
* @param expenseFile
* @param months
*/
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

MonthlyBudget mb;
MonthlyExpenses me;
ofstream toFile;

budgetFile.open("budget.bin", ios::in






share|improve this question











The program will track the monthly budget of the user and will then compare it to the expenses of the user. The data members of the structs will be sent to binary files. Once the results are displayed to the user, the results will be sent to a text file. Also be advised, My teacher is very old schooled so I apologize if some of the methods I've used are out of date.



#include <cstdlib>
#include <limits>
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>
#include <cmath>
#include <cctype>
using namespace std;

struct MonthlyBudget

double housing;
double utilites;
double houseHold;
double transportation;
double food;
double medical;
double insurance;
double entertainment;
double clothinng;
double misc;
;

struct MonthlyExpenses

double housingEx;
double utilitesEx;
double householdEx;
double transportationEx;
double foodEx;
double medicalEx;
double insuranceEx;
double entertainmentEx;
double clothinngEx;
double miscEx;
;

//prototypes declared outside of main so main function logic isn't distorted
void describeProgram();
void getMonths(int& months);
void getMonthlyBudget(fstream& budgetFile, int months);
void getMonthlyExpenses(fstream& expenseFile, int months);
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months);

/**
*
* @return 0
*/
int main()

int months = 1;
fstream budgetFile, expenseFile;

describeProgram();
getMonths(months);
getMonthlyBudget(budgetFile, months);
getMonthlyExpenses(expenseFile, months);
getMonthlyReport(budgetFile, expenseFile, months);

return 0;


/**
*
*/
void describeProgram()

cout<<
"This program will ask you for your desired budget for housing, "
"utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
" of money you spent on such expenses. n First, the program will ask you for how "
"many months you would like analyzed. These numbers entered will be put into "
"a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
"It will then calculate the difference and display the budget, n the expenses, "
"and over/under in a tabular format.n";


/**
*
* @param months
*/
void getMonths(int& months)

cout<<"How many months would you like to have analyzed? ";
cin>>months;

while(cin.fail())

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(),'n');
cout << "Enter valid input ";
cin >> months;




/**
*
* @param budgetFile
* @param months
*/
void getMonthlyBudget(fstream& budgetFile, int months)
ios::binary);
if(budgetFile.fail())

cout<<"Could not find file: budget.bin n";
system("read"); //act as system("PAUSE")


try

do

cout<<"Enter your housing budget for month "<<count<<":n";
cin>>mb.housing;
cout<<"Enter your utilities budget for month "<<count<<":n";
cin>>mb.utilites;
cout<<"Enter your household expense budget for month "<<count<<":n";
cin>>mb.houseHold;
cout<<"Enter your transportation budget for month "<<count<<":n";
cin>>mb.transportation;
cout<<"Enter your food budget for month "<<count<<":n";
cin>>mb.food;
cout<<"Enter your medical budget for month "<<count<<":n";
cin>>mb.medical;
cout<<"Enter your insurance budget for month "<<count<<":n";
cin>>mb.insurance;
cout<<"Enter your entertainment budget for month "<<count<<":n";
cin>>mb.entertainment;
cout<<"Enter your clothing budget for month "<<count<<":n";
cin>>mb.clothinng;
cout<<"Enter your Miscellaneous budget for month "<<count<<":n";
cin>>mb.misc;

budgetFile.write(reinterpret_cast<char*>(&mb),sizeof(mb));
count++;
months --;
while(months != 0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";


budgetFile.close();


/**
*
* @param expenseFile
* @param months
*/
void getMonthlyExpenses(fstream& expenseFile, int months)
ios::binary);
if(expenseFile.fail())

cout<<"Could not open expenses.bin n";
system("read");

try

do

cout<<"Enter your housing expenses for month "<<count<<":n";
cin>>me.housingEx;
cout<<"Enter your utilities expenses for month "<<count<<":n";
cin>>me.utilitesEx;
cout<<"Enter the cost of your house hold expenses for month "<<count<<":n";
cin>>me.householdEx;
cout<<"Enter your transportation expenses for month "<<count<<":n";
cin>>me.transportationEx;
cout<<"Enter your food expenses for month "<<count<<":n";
cin>>me.foodEx;
cout<<"Enter your medical expenses for month "<<count<<":n";
cin>>me.medicalEx;
cout<<"Enter your insurance expenses for month "<<count<<":n";
cin>>me.insuranceEx;
cout<<"Enter your entertainment expenses for month "<<count<<":n";
cin>>me.entertainmentEx;
cout<<"Enter your clothing expenses for month "<<count<<":n";
cin>>me.clothinngEx;
cout<<"Enter your Miscellaneous expenses for month "<<count<<":n";
cin>>me.miscEx;

expenseFile.write(reinterpret_cast<char*>(&me),sizeof(me));
count++;
months--;
while(months!=0);


catch(...)

cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), 'n');

cout<<"Error, invalid input entered.";

expenseFile.close();


/**
*
* @param budgetFile
* @param expenseFile
* @param months
*/
void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

MonthlyBudget mb;
MonthlyExpenses me;
ofstream toFile;

budgetFile.open("budget.bin", ios::in








share|improve this question










share|improve this question




share|improve this question









asked Mar 1 at 1:08









Zachariah

912




912











  • As I am not any expert in cpp, I prefer to post short advices instead of full answer: 1) please make a spot that budget and expenses are the same data structures in essence but only have different purpose. 2) instead of using fixed structs of doubles you can use dictionary-like data structure and iterate over its keys. 3) reading budget and expenses for month from the user is in essence the same but under different name 'budget'/'expenses' and into different target variable.
    – mpasko256
    Mar 1 at 13:31










  • Does your teacher enforce the rather curious use of system("read")?
    – yuri
    Mar 1 at 15:11










  • no, he does however encourage system("PAUSE") which I heard was worse. I just used read because I saw a few forums encouraging that instead. What would be the best option?
    – Zachariah
    Mar 1 at 16:37










  • @Zachariah I don't know about the best but cin.get() seems like a better choice.
    – yuri
    Mar 1 at 16:52

















  • As I am not any expert in cpp, I prefer to post short advices instead of full answer: 1) please make a spot that budget and expenses are the same data structures in essence but only have different purpose. 2) instead of using fixed structs of doubles you can use dictionary-like data structure and iterate over its keys. 3) reading budget and expenses for month from the user is in essence the same but under different name 'budget'/'expenses' and into different target variable.
    – mpasko256
    Mar 1 at 13:31










  • Does your teacher enforce the rather curious use of system("read")?
    – yuri
    Mar 1 at 15:11










  • no, he does however encourage system("PAUSE") which I heard was worse. I just used read because I saw a few forums encouraging that instead. What would be the best option?
    – Zachariah
    Mar 1 at 16:37










  • @Zachariah I don't know about the best but cin.get() seems like a better choice.
    – yuri
    Mar 1 at 16:52
















As I am not any expert in cpp, I prefer to post short advices instead of full answer: 1) please make a spot that budget and expenses are the same data structures in essence but only have different purpose. 2) instead of using fixed structs of doubles you can use dictionary-like data structure and iterate over its keys. 3) reading budget and expenses for month from the user is in essence the same but under different name 'budget'/'expenses' and into different target variable.
– mpasko256
Mar 1 at 13:31




As I am not any expert in cpp, I prefer to post short advices instead of full answer: 1) please make a spot that budget and expenses are the same data structures in essence but only have different purpose. 2) instead of using fixed structs of doubles you can use dictionary-like data structure and iterate over its keys. 3) reading budget and expenses for month from the user is in essence the same but under different name 'budget'/'expenses' and into different target variable.
– mpasko256
Mar 1 at 13:31












Does your teacher enforce the rather curious use of system("read")?
– yuri
Mar 1 at 15:11




Does your teacher enforce the rather curious use of system("read")?
– yuri
Mar 1 at 15:11












no, he does however encourage system("PAUSE") which I heard was worse. I just used read because I saw a few forums encouraging that instead. What would be the best option?
– Zachariah
Mar 1 at 16:37




no, he does however encourage system("PAUSE") which I heard was worse. I just used read because I saw a few forums encouraging that instead. What would be the best option?
– Zachariah
Mar 1 at 16:37












@Zachariah I don't know about the best but cin.get() seems like a better choice.
– yuri
Mar 1 at 16:52





@Zachariah I don't know about the best but cin.get() seems like a better choice.
– yuri
Mar 1 at 16:52











2 Answers
2






active

oldest

votes

















up vote
2
down vote













DRY you code



DRY: Don't repeat yourself



This piece of code is repeated many times:



 toFile<<left<<setw(14)<<"Insurance";
toFile<<right<<setw(11)<<mb.insurance;
toFile<<right<<setw(14)<<me.insuranceEx;
toFile<<right<<setw(14)<<(mb.insurance - me.insuranceEx)<<"n";


This means it is ripe for placing into a function:



 void dump(std::ostream& str, std::string const& title, double value, double valueEx) 
str << left << setw(14) << title
<< right << setw(11) << value
<< right << setw(14) << valueEx
<< right << setw(14) << (value - valueEx)
<< "n";



Don't Write your file in BInary



budgetFile.read(reinterpret_cast<char*>(&mb),sizeof(mb));


The trouble here is that this is very brittle. It will work on your current current compiler with this set of compiler flags on the current OS on the current hardware. Any of the these things changes and your data is likely to break.



Rather stream the data (in a human readable form) to a file. That way it can be read portably across multiple systems without much difficulty.



Also you can version your data. So that if in the future you add (or remove) fields into your structure your code can compensate for this.



struct MonthlyBudget

friend std::ostream& operator<<(std::ostream& strm, MonthlyBudget const& mb)

// Save data to stream.
str << "2 " // version 2
<< mb. housing << " "
<< mb. utilites << " "
// etc
<< "n";
return str;

friend std::istream& operator>>(std::istream& strm, MonthlyBudget& mb)

int version;
str >> version;
if (1 <= version)
// read version 1 fields
str >> mb. housing
>> mb. utilites
// etc

if (2 <= version)
// read version 2 fields
str >> mb.spaceHop;

return str;


;


Use the new String literal



Rather than artificially split the text into multiple strings.



cout<<
"This program will ask you for your desired budget for housing, "
"utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
" of money you spent on such expenses. n First, the program will ask you for how "
"many months you would like analyzed. These numbers entered will be put into "
"a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
"It will then calculate the difference and display the budget, n the expenses, "
"and over/under in a tabular format.n";


You can have very long strings over multiple lines now with C++11 string literals. In this case use the RAW string literal.



std:: cout << 
R"(This program will ask you for your desired budget for housing, utilites, entertainment, etc (8 budget options), and will also ask you for the amount of money you spent on such expenses.
First, the program will ask you for how many months you would like analyzed. These numbers entered will be put into
MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). It will then calculate the difference and display the budget, and over/under in a tabular format.
)";


Basically the RAW string can contain the newline character naturaly. So it is easier to write and format as you would expect.



// Note the: <optional Marker> must be the same at both ends.
// <Data> is the actual string used by the compiler
// and can include all characters you want (or can type).
std::string str = R"<optional Marker>(<Data>)<optional Marker>";





share|improve this answer






























    up vote
    1
    down vote













    Good job breaking up the logic in main() into different functions. However, some improvements can be made.



    Avoid using namespace std



    This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.



    Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.



    Make your code more re-useable / Don't Repeat Yourself (DRY)



    There are places where you've written a lot of code that does essentially the same thing. For example:



    struct MonthlyBudget

    double housing;
    double utilites;
    double houseHold;
    double transportation;
    double food;
    double medical;
    double insurance;
    double entertainment;
    double clothinng;
    double misc;
    ;

    struct MonthlyExpenses

    double housingEx;
    double utilitesEx;
    double householdEx;
    double transportationEx;
    double foodEx;
    double medicalEx;
    double insuranceEx;
    double entertainmentEx;
    double clothinngEx;
    double miscEx;
    ;


    These two structs hold the same categories of data, just that one holds budget data and the other holds expenses data. You can just create one struct (called, e.g. Categories) and use one variable of that type for all the budget data and another variable of that type for all the expenses data. Using one struct for both reduces the amount of code you have to write and, more importantly, reduces the risk of bugs if you need to add or remove categories: instead of having to remember to modify both of your structs you'd only have to modify the one struct.



    Similarly, you have a lot of code like this:



    cout<<"Enter your housing budget for month "<<count<<":n";
    cin>>mb.housing;
    cout<<"Enter your utilities budget for month "<<count<<":n";
    cin>>mb.utilites;


    You can put that into a function that looks like this:



    double get_budget(std::string category_name, int count) 
    double budget = 0;
    std::cout << "Enter your " + category_name + " budget for month " << count << ":n";
    std::cin >> budget;
    // Check for validity of the user input here (e.g. that it is positive).
    return budget;



    You could do the same with the code that acquires the expenses inputs (in fact, you could write one function for both).



    Don't use magic numbers



    You've got a lot of repeated code like this:



    cout<<left<<setw(14)<<"Housing";
    cout<<right<<setw(11)<<mb.housing;
    cout<<right<<setw(14)<<me.housingEx;
    cout<<right<<setw(14)<<(mb.housing - me.housingEx)<<"n";


    Not only is this an opportunity to refactor that code into a function, but the numbers 14 and 11 (arguments to setw() here) appear multiple times in your code with the same purpose. Instead, define a constant (e.g. const int width = 14) and use that constant. This means that if you ever want to change that value you'd only have to change the value of that constant once (the value of the const int width) instead of trying to find the literal 14 all over your program.



    Use meaningful comments



    Your functions are commented as so:



    /**
    *
    * @param budgetFile
    * @param expenseFile
    * @param months
    */
    void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

    // ...



    While it's a good idea to provide documentation on the parameters for your functions, these comments as is are not helpful. They just take up space and are distracting -- after all, I can see the names of the parameters just a few lines below the comments.



    Instead of simply listing the parameter name, provide some explanation of what values it is allowed to take (e.g. months should be between 1 and 12, inclusive), what happens if the parameter value is not valid, etc.






    share|improve this answer





















      Your Answer




      StackExchange.ifUsing("editor", function ()
      return StackExchange.using("mathjaxEditing", function ()
      StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
      StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
      );
      );
      , "mathjax-editing");

      StackExchange.ifUsing("editor", function ()
      StackExchange.using("externalEditor", function ()
      StackExchange.using("snippets", function ()
      StackExchange.snippets.init();
      );
      );
      , "code-snippets");

      StackExchange.ready(function()
      var channelOptions =
      tags: "".split(" "),
      id: "196"
      ;
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function()
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled)
      StackExchange.using("snippets", function()
      createEditor();
      );

      else
      createEditor();

      );

      function createEditor()
      StackExchange.prepareEditor(
      heartbeatType: 'answer',
      convertImagesToLinks: false,
      noModals: false,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      );



      );








       

      draft saved


      draft discarded


















      StackExchange.ready(
      function ()
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f188577%2fmonthlybudget-tracker-that-tracks-users-budget-and-compares-it-to-their-expense%23new-answer', 'question_page');

      );

      Post as a guest






























      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes








      up vote
      2
      down vote













      DRY you code



      DRY: Don't repeat yourself



      This piece of code is repeated many times:



       toFile<<left<<setw(14)<<"Insurance";
      toFile<<right<<setw(11)<<mb.insurance;
      toFile<<right<<setw(14)<<me.insuranceEx;
      toFile<<right<<setw(14)<<(mb.insurance - me.insuranceEx)<<"n";


      This means it is ripe for placing into a function:



       void dump(std::ostream& str, std::string const& title, double value, double valueEx) 
      str << left << setw(14) << title
      << right << setw(11) << value
      << right << setw(14) << valueEx
      << right << setw(14) << (value - valueEx)
      << "n";



      Don't Write your file in BInary



      budgetFile.read(reinterpret_cast<char*>(&mb),sizeof(mb));


      The trouble here is that this is very brittle. It will work on your current current compiler with this set of compiler flags on the current OS on the current hardware. Any of the these things changes and your data is likely to break.



      Rather stream the data (in a human readable form) to a file. That way it can be read portably across multiple systems without much difficulty.



      Also you can version your data. So that if in the future you add (or remove) fields into your structure your code can compensate for this.



      struct MonthlyBudget

      friend std::ostream& operator<<(std::ostream& strm, MonthlyBudget const& mb)

      // Save data to stream.
      str << "2 " // version 2
      << mb. housing << " "
      << mb. utilites << " "
      // etc
      << "n";
      return str;

      friend std::istream& operator>>(std::istream& strm, MonthlyBudget& mb)

      int version;
      str >> version;
      if (1 <= version)
      // read version 1 fields
      str >> mb. housing
      >> mb. utilites
      // etc

      if (2 <= version)
      // read version 2 fields
      str >> mb.spaceHop;

      return str;


      ;


      Use the new String literal



      Rather than artificially split the text into multiple strings.



      cout<<
      "This program will ask you for your desired budget for housing, "
      "utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
      " of money you spent on such expenses. n First, the program will ask you for how "
      "many months you would like analyzed. These numbers entered will be put into "
      "a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
      "It will then calculate the difference and display the budget, n the expenses, "
      "and over/under in a tabular format.n";


      You can have very long strings over multiple lines now with C++11 string literals. In this case use the RAW string literal.



      std:: cout << 
      R"(This program will ask you for your desired budget for housing, utilites, entertainment, etc (8 budget options), and will also ask you for the amount of money you spent on such expenses.
      First, the program will ask you for how many months you would like analyzed. These numbers entered will be put into
      MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). It will then calculate the difference and display the budget, and over/under in a tabular format.
      )";


      Basically the RAW string can contain the newline character naturaly. So it is easier to write and format as you would expect.



      // Note the: <optional Marker> must be the same at both ends.
      // <Data> is the actual string used by the compiler
      // and can include all characters you want (or can type).
      std::string str = R"<optional Marker>(<Data>)<optional Marker>";





      share|improve this answer



























        up vote
        2
        down vote













        DRY you code



        DRY: Don't repeat yourself



        This piece of code is repeated many times:



         toFile<<left<<setw(14)<<"Insurance";
        toFile<<right<<setw(11)<<mb.insurance;
        toFile<<right<<setw(14)<<me.insuranceEx;
        toFile<<right<<setw(14)<<(mb.insurance - me.insuranceEx)<<"n";


        This means it is ripe for placing into a function:



         void dump(std::ostream& str, std::string const& title, double value, double valueEx) 
        str << left << setw(14) << title
        << right << setw(11) << value
        << right << setw(14) << valueEx
        << right << setw(14) << (value - valueEx)
        << "n";



        Don't Write your file in BInary



        budgetFile.read(reinterpret_cast<char*>(&mb),sizeof(mb));


        The trouble here is that this is very brittle. It will work on your current current compiler with this set of compiler flags on the current OS on the current hardware. Any of the these things changes and your data is likely to break.



        Rather stream the data (in a human readable form) to a file. That way it can be read portably across multiple systems without much difficulty.



        Also you can version your data. So that if in the future you add (or remove) fields into your structure your code can compensate for this.



        struct MonthlyBudget

        friend std::ostream& operator<<(std::ostream& strm, MonthlyBudget const& mb)

        // Save data to stream.
        str << "2 " // version 2
        << mb. housing << " "
        << mb. utilites << " "
        // etc
        << "n";
        return str;

        friend std::istream& operator>>(std::istream& strm, MonthlyBudget& mb)

        int version;
        str >> version;
        if (1 <= version)
        // read version 1 fields
        str >> mb. housing
        >> mb. utilites
        // etc

        if (2 <= version)
        // read version 2 fields
        str >> mb.spaceHop;

        return str;


        ;


        Use the new String literal



        Rather than artificially split the text into multiple strings.



        cout<<
        "This program will ask you for your desired budget for housing, "
        "utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
        " of money you spent on such expenses. n First, the program will ask you for how "
        "many months you would like analyzed. These numbers entered will be put into "
        "a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
        "It will then calculate the difference and display the budget, n the expenses, "
        "and over/under in a tabular format.n";


        You can have very long strings over multiple lines now with C++11 string literals. In this case use the RAW string literal.



        std:: cout << 
        R"(This program will ask you for your desired budget for housing, utilites, entertainment, etc (8 budget options), and will also ask you for the amount of money you spent on such expenses.
        First, the program will ask you for how many months you would like analyzed. These numbers entered will be put into
        MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). It will then calculate the difference and display the budget, and over/under in a tabular format.
        )";


        Basically the RAW string can contain the newline character naturaly. So it is easier to write and format as you would expect.



        // Note the: <optional Marker> must be the same at both ends.
        // <Data> is the actual string used by the compiler
        // and can include all characters you want (or can type).
        std::string str = R"<optional Marker>(<Data>)<optional Marker>";





        share|improve this answer

























          up vote
          2
          down vote










          up vote
          2
          down vote









          DRY you code



          DRY: Don't repeat yourself



          This piece of code is repeated many times:



           toFile<<left<<setw(14)<<"Insurance";
          toFile<<right<<setw(11)<<mb.insurance;
          toFile<<right<<setw(14)<<me.insuranceEx;
          toFile<<right<<setw(14)<<(mb.insurance - me.insuranceEx)<<"n";


          This means it is ripe for placing into a function:



           void dump(std::ostream& str, std::string const& title, double value, double valueEx) 
          str << left << setw(14) << title
          << right << setw(11) << value
          << right << setw(14) << valueEx
          << right << setw(14) << (value - valueEx)
          << "n";



          Don't Write your file in BInary



          budgetFile.read(reinterpret_cast<char*>(&mb),sizeof(mb));


          The trouble here is that this is very brittle. It will work on your current current compiler with this set of compiler flags on the current OS on the current hardware. Any of the these things changes and your data is likely to break.



          Rather stream the data (in a human readable form) to a file. That way it can be read portably across multiple systems without much difficulty.



          Also you can version your data. So that if in the future you add (or remove) fields into your structure your code can compensate for this.



          struct MonthlyBudget

          friend std::ostream& operator<<(std::ostream& strm, MonthlyBudget const& mb)

          // Save data to stream.
          str << "2 " // version 2
          << mb. housing << " "
          << mb. utilites << " "
          // etc
          << "n";
          return str;

          friend std::istream& operator>>(std::istream& strm, MonthlyBudget& mb)

          int version;
          str >> version;
          if (1 <= version)
          // read version 1 fields
          str >> mb. housing
          >> mb. utilites
          // etc

          if (2 <= version)
          // read version 2 fields
          str >> mb.spaceHop;

          return str;


          ;


          Use the new String literal



          Rather than artificially split the text into multiple strings.



          cout<<
          "This program will ask you for your desired budget for housing, "
          "utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
          " of money you spent on such expenses. n First, the program will ask you for how "
          "many months you would like analyzed. These numbers entered will be put into "
          "a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
          "It will then calculate the difference and display the budget, n the expenses, "
          "and over/under in a tabular format.n";


          You can have very long strings over multiple lines now with C++11 string literals. In this case use the RAW string literal.



          std:: cout << 
          R"(This program will ask you for your desired budget for housing, utilites, entertainment, etc (8 budget options), and will also ask you for the amount of money you spent on such expenses.
          First, the program will ask you for how many months you would like analyzed. These numbers entered will be put into
          MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). It will then calculate the difference and display the budget, and over/under in a tabular format.
          )";


          Basically the RAW string can contain the newline character naturaly. So it is easier to write and format as you would expect.



          // Note the: <optional Marker> must be the same at both ends.
          // <Data> is the actual string used by the compiler
          // and can include all characters you want (or can type).
          std::string str = R"<optional Marker>(<Data>)<optional Marker>";





          share|improve this answer















          DRY you code



          DRY: Don't repeat yourself



          This piece of code is repeated many times:



           toFile<<left<<setw(14)<<"Insurance";
          toFile<<right<<setw(11)<<mb.insurance;
          toFile<<right<<setw(14)<<me.insuranceEx;
          toFile<<right<<setw(14)<<(mb.insurance - me.insuranceEx)<<"n";


          This means it is ripe for placing into a function:



           void dump(std::ostream& str, std::string const& title, double value, double valueEx) 
          str << left << setw(14) << title
          << right << setw(11) << value
          << right << setw(14) << valueEx
          << right << setw(14) << (value - valueEx)
          << "n";



          Don't Write your file in BInary



          budgetFile.read(reinterpret_cast<char*>(&mb),sizeof(mb));


          The trouble here is that this is very brittle. It will work on your current current compiler with this set of compiler flags on the current OS on the current hardware. Any of the these things changes and your data is likely to break.



          Rather stream the data (in a human readable form) to a file. That way it can be read portably across multiple systems without much difficulty.



          Also you can version your data. So that if in the future you add (or remove) fields into your structure your code can compensate for this.



          struct MonthlyBudget

          friend std::ostream& operator<<(std::ostream& strm, MonthlyBudget const& mb)

          // Save data to stream.
          str << "2 " // version 2
          << mb. housing << " "
          << mb. utilites << " "
          // etc
          << "n";
          return str;

          friend std::istream& operator>>(std::istream& strm, MonthlyBudget& mb)

          int version;
          str >> version;
          if (1 <= version)
          // read version 1 fields
          str >> mb. housing
          >> mb. utilites
          // etc

          if (2 <= version)
          // read version 2 fields
          str >> mb.spaceHop;

          return str;


          ;


          Use the new String literal



          Rather than artificially split the text into multiple strings.



          cout<<
          "This program will ask you for your desired budget for housing, "
          "utilites, entertainment, n etc (8 budget options), and will also ask you for the amount"
          " of money you spent on such expenses. n First, the program will ask you for how "
          "many months you would like analyzed. These numbers entered will be put into "
          "a n MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). "
          "It will then calculate the difference and display the budget, n the expenses, "
          "and over/under in a tabular format.n";


          You can have very long strings over multiple lines now with C++11 string literals. In this case use the RAW string literal.



          std:: cout << 
          R"(This program will ask you for your desired budget for housing, utilites, entertainment, etc (8 budget options), and will also ask you for the amount of money you spent on such expenses.
          First, the program will ask you for how many months you would like analyzed. These numbers entered will be put into
          MonthlyBudget struct and sent to two external files (budget.bin ad expenses.bin). It will then calculate the difference and display the budget, and over/under in a tabular format.
          )";


          Basically the RAW string can contain the newline character naturaly. So it is easier to write and format as you would expect.



          // Note the: <optional Marker> must be the same at both ends.
          // <Data> is the actual string used by the compiler
          // and can include all characters you want (or can type).
          std::string str = R"<optional Marker>(<Data>)<optional Marker>";






          share|improve this answer















          share|improve this answer



          share|improve this answer








          edited Mar 1 at 19:01


























          answered Mar 1 at 18:40









          Martin York

          70.9k481244




          70.9k481244






















              up vote
              1
              down vote













              Good job breaking up the logic in main() into different functions. However, some improvements can be made.



              Avoid using namespace std



              This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.



              Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.



              Make your code more re-useable / Don't Repeat Yourself (DRY)



              There are places where you've written a lot of code that does essentially the same thing. For example:



              struct MonthlyBudget

              double housing;
              double utilites;
              double houseHold;
              double transportation;
              double food;
              double medical;
              double insurance;
              double entertainment;
              double clothinng;
              double misc;
              ;

              struct MonthlyExpenses

              double housingEx;
              double utilitesEx;
              double householdEx;
              double transportationEx;
              double foodEx;
              double medicalEx;
              double insuranceEx;
              double entertainmentEx;
              double clothinngEx;
              double miscEx;
              ;


              These two structs hold the same categories of data, just that one holds budget data and the other holds expenses data. You can just create one struct (called, e.g. Categories) and use one variable of that type for all the budget data and another variable of that type for all the expenses data. Using one struct for both reduces the amount of code you have to write and, more importantly, reduces the risk of bugs if you need to add or remove categories: instead of having to remember to modify both of your structs you'd only have to modify the one struct.



              Similarly, you have a lot of code like this:



              cout<<"Enter your housing budget for month "<<count<<":n";
              cin>>mb.housing;
              cout<<"Enter your utilities budget for month "<<count<<":n";
              cin>>mb.utilites;


              You can put that into a function that looks like this:



              double get_budget(std::string category_name, int count) 
              double budget = 0;
              std::cout << "Enter your " + category_name + " budget for month " << count << ":n";
              std::cin >> budget;
              // Check for validity of the user input here (e.g. that it is positive).
              return budget;



              You could do the same with the code that acquires the expenses inputs (in fact, you could write one function for both).



              Don't use magic numbers



              You've got a lot of repeated code like this:



              cout<<left<<setw(14)<<"Housing";
              cout<<right<<setw(11)<<mb.housing;
              cout<<right<<setw(14)<<me.housingEx;
              cout<<right<<setw(14)<<(mb.housing - me.housingEx)<<"n";


              Not only is this an opportunity to refactor that code into a function, but the numbers 14 and 11 (arguments to setw() here) appear multiple times in your code with the same purpose. Instead, define a constant (e.g. const int width = 14) and use that constant. This means that if you ever want to change that value you'd only have to change the value of that constant once (the value of the const int width) instead of trying to find the literal 14 all over your program.



              Use meaningful comments



              Your functions are commented as so:



              /**
              *
              * @param budgetFile
              * @param expenseFile
              * @param months
              */
              void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

              // ...



              While it's a good idea to provide documentation on the parameters for your functions, these comments as is are not helpful. They just take up space and are distracting -- after all, I can see the names of the parameters just a few lines below the comments.



              Instead of simply listing the parameter name, provide some explanation of what values it is allowed to take (e.g. months should be between 1 and 12, inclusive), what happens if the parameter value is not valid, etc.






              share|improve this answer

























                up vote
                1
                down vote













                Good job breaking up the logic in main() into different functions. However, some improvements can be made.



                Avoid using namespace std



                This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.



                Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.



                Make your code more re-useable / Don't Repeat Yourself (DRY)



                There are places where you've written a lot of code that does essentially the same thing. For example:



                struct MonthlyBudget

                double housing;
                double utilites;
                double houseHold;
                double transportation;
                double food;
                double medical;
                double insurance;
                double entertainment;
                double clothinng;
                double misc;
                ;

                struct MonthlyExpenses

                double housingEx;
                double utilitesEx;
                double householdEx;
                double transportationEx;
                double foodEx;
                double medicalEx;
                double insuranceEx;
                double entertainmentEx;
                double clothinngEx;
                double miscEx;
                ;


                These two structs hold the same categories of data, just that one holds budget data and the other holds expenses data. You can just create one struct (called, e.g. Categories) and use one variable of that type for all the budget data and another variable of that type for all the expenses data. Using one struct for both reduces the amount of code you have to write and, more importantly, reduces the risk of bugs if you need to add or remove categories: instead of having to remember to modify both of your structs you'd only have to modify the one struct.



                Similarly, you have a lot of code like this:



                cout<<"Enter your housing budget for month "<<count<<":n";
                cin>>mb.housing;
                cout<<"Enter your utilities budget for month "<<count<<":n";
                cin>>mb.utilites;


                You can put that into a function that looks like this:



                double get_budget(std::string category_name, int count) 
                double budget = 0;
                std::cout << "Enter your " + category_name + " budget for month " << count << ":n";
                std::cin >> budget;
                // Check for validity of the user input here (e.g. that it is positive).
                return budget;



                You could do the same with the code that acquires the expenses inputs (in fact, you could write one function for both).



                Don't use magic numbers



                You've got a lot of repeated code like this:



                cout<<left<<setw(14)<<"Housing";
                cout<<right<<setw(11)<<mb.housing;
                cout<<right<<setw(14)<<me.housingEx;
                cout<<right<<setw(14)<<(mb.housing - me.housingEx)<<"n";


                Not only is this an opportunity to refactor that code into a function, but the numbers 14 and 11 (arguments to setw() here) appear multiple times in your code with the same purpose. Instead, define a constant (e.g. const int width = 14) and use that constant. This means that if you ever want to change that value you'd only have to change the value of that constant once (the value of the const int width) instead of trying to find the literal 14 all over your program.



                Use meaningful comments



                Your functions are commented as so:



                /**
                *
                * @param budgetFile
                * @param expenseFile
                * @param months
                */
                void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

                // ...



                While it's a good idea to provide documentation on the parameters for your functions, these comments as is are not helpful. They just take up space and are distracting -- after all, I can see the names of the parameters just a few lines below the comments.



                Instead of simply listing the parameter name, provide some explanation of what values it is allowed to take (e.g. months should be between 1 and 12, inclusive), what happens if the parameter value is not valid, etc.






                share|improve this answer























                  up vote
                  1
                  down vote










                  up vote
                  1
                  down vote









                  Good job breaking up the logic in main() into different functions. However, some improvements can be made.



                  Avoid using namespace std



                  This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.



                  Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.



                  Make your code more re-useable / Don't Repeat Yourself (DRY)



                  There are places where you've written a lot of code that does essentially the same thing. For example:



                  struct MonthlyBudget

                  double housing;
                  double utilites;
                  double houseHold;
                  double transportation;
                  double food;
                  double medical;
                  double insurance;
                  double entertainment;
                  double clothinng;
                  double misc;
                  ;

                  struct MonthlyExpenses

                  double housingEx;
                  double utilitesEx;
                  double householdEx;
                  double transportationEx;
                  double foodEx;
                  double medicalEx;
                  double insuranceEx;
                  double entertainmentEx;
                  double clothinngEx;
                  double miscEx;
                  ;


                  These two structs hold the same categories of data, just that one holds budget data and the other holds expenses data. You can just create one struct (called, e.g. Categories) and use one variable of that type for all the budget data and another variable of that type for all the expenses data. Using one struct for both reduces the amount of code you have to write and, more importantly, reduces the risk of bugs if you need to add or remove categories: instead of having to remember to modify both of your structs you'd only have to modify the one struct.



                  Similarly, you have a lot of code like this:



                  cout<<"Enter your housing budget for month "<<count<<":n";
                  cin>>mb.housing;
                  cout<<"Enter your utilities budget for month "<<count<<":n";
                  cin>>mb.utilites;


                  You can put that into a function that looks like this:



                  double get_budget(std::string category_name, int count) 
                  double budget = 0;
                  std::cout << "Enter your " + category_name + " budget for month " << count << ":n";
                  std::cin >> budget;
                  // Check for validity of the user input here (e.g. that it is positive).
                  return budget;



                  You could do the same with the code that acquires the expenses inputs (in fact, you could write one function for both).



                  Don't use magic numbers



                  You've got a lot of repeated code like this:



                  cout<<left<<setw(14)<<"Housing";
                  cout<<right<<setw(11)<<mb.housing;
                  cout<<right<<setw(14)<<me.housingEx;
                  cout<<right<<setw(14)<<(mb.housing - me.housingEx)<<"n";


                  Not only is this an opportunity to refactor that code into a function, but the numbers 14 and 11 (arguments to setw() here) appear multiple times in your code with the same purpose. Instead, define a constant (e.g. const int width = 14) and use that constant. This means that if you ever want to change that value you'd only have to change the value of that constant once (the value of the const int width) instead of trying to find the literal 14 all over your program.



                  Use meaningful comments



                  Your functions are commented as so:



                  /**
                  *
                  * @param budgetFile
                  * @param expenseFile
                  * @param months
                  */
                  void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

                  // ...



                  While it's a good idea to provide documentation on the parameters for your functions, these comments as is are not helpful. They just take up space and are distracting -- after all, I can see the names of the parameters just a few lines below the comments.



                  Instead of simply listing the parameter name, provide some explanation of what values it is allowed to take (e.g. months should be between 1 and 12, inclusive), what happens if the parameter value is not valid, etc.






                  share|improve this answer













                  Good job breaking up the logic in main() into different functions. However, some improvements can be made.



                  Avoid using namespace std



                  This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.



                  Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.



                  Make your code more re-useable / Don't Repeat Yourself (DRY)



                  There are places where you've written a lot of code that does essentially the same thing. For example:



                  struct MonthlyBudget

                  double housing;
                  double utilites;
                  double houseHold;
                  double transportation;
                  double food;
                  double medical;
                  double insurance;
                  double entertainment;
                  double clothinng;
                  double misc;
                  ;

                  struct MonthlyExpenses

                  double housingEx;
                  double utilitesEx;
                  double householdEx;
                  double transportationEx;
                  double foodEx;
                  double medicalEx;
                  double insuranceEx;
                  double entertainmentEx;
                  double clothinngEx;
                  double miscEx;
                  ;


                  These two structs hold the same categories of data, just that one holds budget data and the other holds expenses data. You can just create one struct (called, e.g. Categories) and use one variable of that type for all the budget data and another variable of that type for all the expenses data. Using one struct for both reduces the amount of code you have to write and, more importantly, reduces the risk of bugs if you need to add or remove categories: instead of having to remember to modify both of your structs you'd only have to modify the one struct.



                  Similarly, you have a lot of code like this:



                  cout<<"Enter your housing budget for month "<<count<<":n";
                  cin>>mb.housing;
                  cout<<"Enter your utilities budget for month "<<count<<":n";
                  cin>>mb.utilites;


                  You can put that into a function that looks like this:



                  double get_budget(std::string category_name, int count) 
                  double budget = 0;
                  std::cout << "Enter your " + category_name + " budget for month " << count << ":n";
                  std::cin >> budget;
                  // Check for validity of the user input here (e.g. that it is positive).
                  return budget;



                  You could do the same with the code that acquires the expenses inputs (in fact, you could write one function for both).



                  Don't use magic numbers



                  You've got a lot of repeated code like this:



                  cout<<left<<setw(14)<<"Housing";
                  cout<<right<<setw(11)<<mb.housing;
                  cout<<right<<setw(14)<<me.housingEx;
                  cout<<right<<setw(14)<<(mb.housing - me.housingEx)<<"n";


                  Not only is this an opportunity to refactor that code into a function, but the numbers 14 and 11 (arguments to setw() here) appear multiple times in your code with the same purpose. Instead, define a constant (e.g. const int width = 14) and use that constant. This means that if you ever want to change that value you'd only have to change the value of that constant once (the value of the const int width) instead of trying to find the literal 14 all over your program.



                  Use meaningful comments



                  Your functions are commented as so:



                  /**
                  *
                  * @param budgetFile
                  * @param expenseFile
                  * @param months
                  */
                  void getMonthlyReport(fstream& budgetFile, fstream& expenseFile, int months)

                  // ...



                  While it's a good idea to provide documentation on the parameters for your functions, these comments as is are not helpful. They just take up space and are distracting -- after all, I can see the names of the parameters just a few lines below the comments.



                  Instead of simply listing the parameter name, provide some explanation of what values it is allowed to take (e.g. months should be between 1 and 12, inclusive), what happens if the parameter value is not valid, etc.







                  share|improve this answer













                  share|improve this answer



                  share|improve this answer











                  answered Mar 1 at 14:56









                  Null

                  8592920




                  8592920






















                       

                      draft saved


                      draft discarded


























                       


                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function ()
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f188577%2fmonthlybudget-tracker-that-tracks-users-budget-and-compares-it-to-their-expense%23new-answer', 'question_page');

                      );

                      Post as a guest













































































                      Popular posts from this blog

                      Greedy Best First Search implementation in Rust

                      Function to Return a JSON Like Objects Using VBA Collections and Arrays

                      C++11 CLH Lock Implementation