C++ code works, but seems very inefficient

Question

I'm currently learning C++ and got my code to do everything I want it to but it seems as if the code is not very efficient because I basically doubled the code from my output console so it shows up in a text file. If you can, could you explain what I am doing wrong and what would you suggest I do in order to make the code more efficient. (Also column one needs to be left justify and column two has to be right justify in both console and text file which I believe I did correctly.)

/*  Description: This program calculates and prints the monthly paycheck for an employee (Output in command prompt and .txt file).*/

#include <iostream>
#include <string>
#include <iomanip>
#include <fstream>

using namespace std;        
char name[256];
double gross;
double fiTax;
double sTax;
double ssTax;
double mediTax;
double pPlan;
double hInsurance;
double tax;
double total;

int main() {  
    std::cout << "Please enter your name: ";
    std::cin.getline(name, 256);        
    cout << "Please enter your gross amount: ";
    cin >> gross;            
    std::cout << std::fixed;    
    std::cout << std::setprecision(2);    
    fiTax = gross * .15;
    sTax = gross * .035;
    ssTax = gross * .0575;
    mediTax = gross * .0275;
    pPlan = gross * .05;
    hInsurance = 75;

    tax = fiTax + sTax + ssTax + mediTax + pPlan + hInsurance;    
    total = gross - tax;    
    system("cls");

    ofstream file;
    file << std::fixed << std::setprecision(2);
    file.open("report.txt");    

    cout << left<< setw(28) << name << endl;
    file << left << setw(28) << name << endl;

    cout << left << setw(28) << "Gross Amount: ............ $";
    cout << right << setw(7) << gross << endl;

    file << left << setw(28) << "Gross Amount: ............ $";
    file << right << setw(7) << gross << endl;

    cout << left << setw(28) << "Federal Tax: ............. $";
    cout << right << setw(7) << fiTax << endl;

    file << left << setw(28) << "Federal Tax: ............. $";
    file << right << setw(7) << fiTax << endl;

    cout << left << setw(28) << "State Tax: ............... $";
    cout << right << setw(7) << sTax << endl;

    file << left << setw(28) << "State Tax: ............... $";
    file << right << setw(7) << sTax << endl;

    cout << left << setw(28) << "Social Security Tax: ..... $";
    cout << right << setw(7) << ssTax << endl;

    file << left << setw(28) << "Social Security Tax: ..... $";
    file << right << setw(7) << ssTax << endl;

    cout << left << setw(28) << "Medicare/medicaid Tax: ... $";
    cout << right << setw(7) << mediTax << endl;

    file << left << setw(28) << "Medicare/medicaid Tax: ... $";
    file << right << setw(7) << mediTax << endl;

    cout << left << setw(28) << "Pension Plan: ............ $";
    cout << right << setw(7) << pPlan << endl;

    file << left << setw(28) << "Pension Plan: ............ $";
    file << right << setw(7) << pPlan << endl;

    cout << left << setw(28) << "Health Insurance: ........ $";
    cout << right << setw(7) << hInsurance << endl;

    file << left << setw(28) << "Health Insurance: ........ $";
    file << right << setw(7) << hInsurance << endl;

    cout << left << setw(28) << "Net Pay: ................. $";
    cout << right << setw(7) << total << endl;

    file << left << setw(28) << "Net Pay: ................. $";
    file << right << setw(7) << total << endl;

    file.close();        
    return 0;
}

Answer 1

One way you can reduce the redundancy in your code is to create a function to do the printing. The C++ output stream classes are all subclasses of std::ostream. This means you can write a function that accepts std::ostream& (reference to std::ostream) and pass it any output stream like std::cout or an std::ofstream.

#include <iostream>
#include <string>
#include <iomanip>
#include <fstream>

using namespace std;
char name[256];
double gross;
double fiTax;
double sTax;
double ssTax;
double mediTax;
double pPlan;
double hInsurance;
double tax;
double total;

// function prints info to any std::ostream
void print_to(std::ostream& out)
{
    out << left<< setw(28) << name << endl;

    out << left << setw(28) << "Gross Amount: ............ $";
    out << right << setw(7) << gross << endl;

    out << left << setw(28) << "Federal Tax: ............. $";
    out << right << setw(7) << fiTax << endl;

    out << left << setw(28) << "State Tax: ............... $";
    out << right << setw(7) << sTax << endl;

    out << left << setw(28) << "Social Security Tax: ..... $";
    out << right << setw(7) << ssTax << endl;

    out << left << setw(28) << "Medicare/medicaid Tax: ... $";
    out << right << setw(7) << mediTax << endl;

    out << left << setw(28) << "Pension Plan: ............ $";
    out << right << setw(7) << pPlan << endl;

    out << left << setw(28) << "Health Insurance: ........ $";
    out << right << setw(7) << hInsurance << endl;

    out << left << setw(28) << "Net Pay: ................. $";
    out << right << setw(7) << total << endl;
}

int main() {
    std::cout << "Please enter your name: ";
    std::cin.getline(name, 256);
    cout << "Please enter your gross amount: ";
    cin >> gross;
    std::cout << std::fixed;
    std::cout << std::setprecision(2);
    fiTax = gross * .15;
    sTax = gross * .035;
    ssTax = gross * .0575;
    mediTax = gross * .0275;
    pPlan = gross * .05;
    hInsurance = 75;

    tax = fiTax + sTax + ssTax + mediTax + pPlan + hInsurance;
    total = gross - tax;
    system("cls");

    // an std::ofstream is a sub-class of std::ostream
    ofstream file;
    file << std::fixed << std::setprecision(2);
    file.open("report.txt");
    print_to(file);
    file.close();

    // an std::cout is a sub-class of std::ostream
    print_to(std::cout);

    return 0;
}

Answer 2

1. Your suspicions of inefficiency are completely unwarranted and irrelevant. Such a trivial, I/O bound code has absolutely no need for optimization, and if you write the whole I/O subsystem from scratch, any performance gain will be negligible and meaningless.

2. You don't have a target efficiency metric, or any efficiency measurements. In other words, you neither know how fast (or slow) your code is, nor how fast it should be. Without measurements and targets, all optimization is useless (or at best, very wasteful.)

3. Your code could look better. Whether the extra empty lines was your work or a side-effect of pasting the code here, you just never bothered to remove them. You should keep in mind that the appearance of your code matters. (Update: I see that you've fixed this. Kudos!)

4. Please don't use global variables, unless you have some experience and you judge that you do need them. In this case, you don't. If you changed you variables' scopes to local, remember to initialize them as well.

5. Don't use char arrays to represent strings. Use std::strings. They have all the array functionality and much more, and are much safer and much more convenient.

6. One way to save yourself some typing/copy+pasting and eliminate some redundancy in your code (which is very bad) is to use the parent class of both the std::cout object's type and the std::ofstream type which is std::ostream. You can write functions that take an object of this type as a parameter and only once write out what you want to write out, then you will call these functions twice: once with std::cout and once with your file.

All these points aside, I hope you'll remember this: don't worry about performance and optimization unless you can objectively prove that it's a problem.

(Sorry about the tone of this response; the OP said he is a beginner and that put me in a lecturing mood!)

Update: You can write a function like this:

void LineOut (std::ostream & os, std::string const & entry, double value)
{
    int dots = 28 - 2 - int(entry.size()); // 2 for ": "
    if (dots < 0) dots = 0;
    os << entry << ": " << std::string('.', dots) << "$" << value << std::endl;
}

// Call it like this:
LineOut(std::cout, "Gross Amount", gross);
LineOut(     file, "Gross Amount", gross);

Now you'll call this function twice for each line of output: once for cout and once for your file.

There are obviously other ways, and better ways, but I doubt that they are worth it for this small a project.

Answer 3

1. You could write a function which will print the values in file and output.

   void WriteToFileAndOutput(const double &val, const string &s, ofstream &fname) {
   cout << left << setw(28) << s;
   cout << right << setw(7) << val << endl;
   
   fname << left << setw(28) << s;
   fname << right << setw(7) << val << endl;
   }
   //You can use it as
   WriteToFileAndOutput(gross, "Gross Amount: ............ $", file);
   WriteToFileAndOutput(fiTax, "Federal Tax: ............. $", file); 
   

2. C++ has built-in string datatype, use it instead of char arrays.

3. If you have included using namespace std then you don't have to specify again that cout, cin etc are from std namespace.

4. Do not use global variables.

---

Just for example:

#include <iostream>
#include <string>
#include <iomanip>
#include <fstream>

using namespace std;

void WriteToFileAndOutput(const double &val, const string &s, ofstream &fname) {
    cout << left << setw(28) << s;
    cout << right << setw(7) << val << endl;

    fname << left << setw(28) << s;
    fname << right << setw(7) << val << endl;
}

int main() {

    string name;
    double gross, fiTax, sTax, ssTax, mediTax, pPlan, hInsurance, tax, total;
    cout << "Please enter your name: ";
    getline(cin,name);
    cout << "Please enter your gross amount: ";
    cin >> gross;

    cout << fixed;
    cout << setprecision(2);

    fiTax = gross * .15;
    sTax = gross * .035;
    ssTax = gross * .0575;
    mediTax = gross * .0275;
    pPlan = gross * .05;
    hInsurance = 75;
    tax = fiTax + sTax + ssTax + mediTax + pPlan + hInsurance;
    total = gross - tax;

    ofstream file;

    file << fixed << setprecision(2);
    file.open("report.txt");

    cout << left << setw(28) << name << endl;
    file << left << setw(28) << name << endl; 

    WriteToFileAndOutput(gross, "Gross Amount: ............ $", file);
    WriteToFileAndOutput(fiTax, "Federal Tax: ............. $", file);
    WriteToFileAndOutput(sTax,  "State Tax: ............... $", file);
    WriteToFileAndOutput(ssTax, "Social Security Tax: ..... $", file);
    WriteToFileAndOutput(mediTax, "Medicare/medicaid Tax: ... $", file);
    WriteToFileAndOutput(pPlan, "Pension Plan: ............ $", file);
    WriteToFileAndOutput(hInsurance, "Health Insurance: ........ $", file);
    WriteToFileAndOutput(total, "Net Pay: ................. $", file);

    file.close();
    return 0;
}