code review, homewrk: 114 lines, beginr polymorphism, factory method

[wretch]

Well since most of you program all day, the last thing you probably want to do is look at code. But if you are bored to death, I'm taking a Design Patterns class using C++, and I am trying to learn C++ (coming from C) and also trying to learn design patterns (factory method in this case). If you feel like tearing me apart, please have at it. I have listed the assignment, and my solution (which I have already turned in) at the bottom (Visual C++ 2008 Express). It runs, produces good output (I think) but I don't know about my C++ style.

Any feedback you can give me would be appreciated, even if just glancing at my code and seeing if something looks extremely amateurish.

P.S. How do I format code for posting?

=================== the assignment =======================================

Objectives: the assignment uses
• inheritance,
• polymorphism,
• virtual functions,
• protected data members,
• files streams,
• operator overloading,
• the “Factory Method” design pattern

The Jones Company wants a single computer program that performs payroll calculation for salaried, hourly, and commissioned sales personnel. To minimize coding and maintenance, it is decided to combine common characteristics of all employees in to a base class named Employee. From it are to be derived three classes: Salaried, Hourly, and Commission. Each of these classes contains data specific to the employee type. Salaried has a double variable named salary. Hourly has doubles rate and hours while Commission has a double named sales. Employee has a char[12] called EmpID, a char[5] named DeptID and a double called net, representing the net pay of all derived classes.

Each class is responsible for reading and writing its own data. That means that Employee must read and write EmpID and DeptID, Salary must read and write salary and so on. The ONLY exception to this rule is “net” from Employee. The derived classes must both calculate the correct value for net and print it as part of their output.

In “main”, you are allowed to create ONE object only. In fact, in the entire program, you may instantiate only ONE object. Using polymorphism, share the object for all employees regardless of type. Write a “Factory Method” to create the objects to assigned to “the” object.

All input is from a data file. The file stream must be passed to each class that needs to read from it. Remember that the file variable should be a reference, otherwise the file pointer will not progress and you program will loop infinitely. On the back is a sample file and output.

Virtual functions for reading, calculation and writing are necessary. This problem can be solved without using virtual functions. However, you will receive no credit for it and I will think poorly of you.

Here is a sample input file for this program.

1 23938475683 1111 62000
2 30495848484 2222 40 21.5
3 40394833333 3333 30000
2 48584842283 2232 45 10
2 38485849494 2222 42.5 15
1 34345564433 1111 56000
3 98605050393 3333 12000
3 00033432738 3333 17000

The first value is a code representing the employee type: 1 is salaried, 2 is hourly, and 3 is commission. The next column is the employee ID, the next the department ID of the employee. The next column varies depending on the employee type. For salaried employees it is their salary per annum. For hourly employees it is the number of hours to be paid. Overtime (1.5) is paid for all time over 40 hours. Following the hours in column five is the rate of pay for the employee. For commission employees the fourth column is the amount of sales for the pay period. Commission is paid on a graduated basis: 0 to 15000 earns 12%, greater than 15000 to 25000 earns 15% and greater than 25000 earns 20%. So for $30,000 the pay is 15000 * 12% + 10000 * 15% + 5000 * 20%.

Notice that only hourly employees have five inputs. The others have four. This is easily accomplished using separate functions for each.

Here is a sample output based on the input above.

Employee #23938475683 Dept# 1111 Net pay: $5166.67 Salary:$62000
Employee #30495848484 Dept# 2222 Net pay: $860 Hours:40 Rate:$21.5
Employee #40394833333 Dept# 3333 Net pay: $4300 Sales:$30000
Employee #48584842283 Dept# 2232 Net pay: $475 Hours:45 Rate:$10
Employee #38485849494 Dept# 2222 Net pay: $656.25 Hours:42.5 Rate:$15
Employee #34345564433 Dept# 1111 Net pay: $4666.67 Salary:$56000
Employee #98605050393 Dept# 3333 Net pay: $1440 Sales:$12000
Employee #00033432738 Dept# 3333 Net pay: $2100 Sales:$17000


======================= my solution =========================================

#include <iostream>
#include <fstream>
#include <stdio.h>
#include <math.h>
#include <algorithm>

using namespace std;

class Employee {
public:
	char outRecord[120];
	char EmpID[12];
	char DeptID[5];
	double NetPay;
	virtual void readWageInfoFromFile(ifstream&, ofstream&)=0;
};

class Salaried : public Employee {
public:
	double yearlySalary;

	void readWageInfoFromFile(ifstream& inFile, ofstream& outFile)
	{
		inFile >> EmpID >>  DeptID >> yearlySalary;
		cout << "just read Salaried  \t" << EmpID << DeptID << yearlySalary;
		NetPay = .833333333*yearlySalary;
		sprintf(outRecord, "Employee #%s Dept #%s NetPay: %9.2f yearlySalary: %9.2f \n",
			EmpID, DeptID, NetPay, yearlySalary);
		cout << outRecord;
		outFile << outRecord;
	}
};

class Hourly : public Employee {
public:
	double Hours, HourlyRate;
	void readWageInfoFromFile(ifstream& inFile, ofstream& outFile)
	{
		inFile >> EmpID >>  DeptID >> Hours >> HourlyRate;
		cout << "just read Hourly      \t" << EmpID <<  DeptID << Hours << HourlyRate;
		NetPay = Hours*HourlyRate + max(0.0,Hours-40.0)*0.5*HourlyRate;
		sprintf(outRecord, "Employee #%s Dept #%s NetPay: %9.2f   HourlyRate: %9.2f Hours: %4.1f \n",
			EmpID, DeptID, NetPay, HourlyRate, Hours);
		cout << outRecord;
		outFile << outRecord;
	}
};

class Commissioned : public Employee {
public:
	double Commission;

	double netFromCommission(double Sales){
		double netFromCom = 0;
		if (Sales <= 15000.0 && Sales >= 0.0) {
			netFromCom = .12*Sales;}
		else if (Sales <= 25000.0) {
			netFromCom = .12*15000.0 + .15*(Sales-15000.0);}
		else if (Sales > 25000.0) {
			netFromCom = .12*15000.0 + .15*10000.0 + .20*(Sales-25000.0);}
		else
			cout << "Invalid Sales: " << Sales;
		return netFromCom;
	}
	void readWageInfoFromFile(ifstream& inFile, ofstream& outFile)
	{
		inFile >> EmpID >>  DeptID >> Commission;
		cout << "just read Commissioned \t" << EmpID <<  DeptID << Commission;
		NetPay = netFromCommission(Commission);
		sprintf(outRecord, "Employee #%s Dept #%s NetPay: %9.2f        Sales: %9.2f \n",
			EmpID, DeptID, NetPay, Commission);
		cout << outRecord;
		outFile << outRecord;
	}

};

Employee* employeeFactory(int type) {
	if (type==1) return new Salaried;
	if (type==2) return new Hourly;
	if (type==3) return new Commissioned;
}

int main()
{
	Employee* empPtr;
	int type;

	ofstream outFile("OneOutput.txt"); // output, normal file
	if(!outFile) {
		cout << "Cannot open outfile: OneOutput.txt \n";
		return 1;
	}

	ifstream inFile("OneInput.txt"); // input
	if(!inFile) {
		cout << "Cannot open input file: 'OneInput.txt' " << endl;
		return 1;
	}
	inFile >> type;  //must read at least once before checking eof()
	while (!inFile.eof( )) //if not at end of file, continue...
	{
		//inFile.getline(lineread, 10); // Read line by 
		if (type!=1 && type!=2 && type!=3) {
			cout << "Invalid employee type: " << type;
			return(1); }
		empPtr = employeeFactory(type);
		empPtr->readWageInfoFromFile(inFile, outFile);
		inFile >> type;
	}
	inFile.close();
	outFile.close();
	return 0;
}

============================= my output ===================================
Employee #23938475683 Dept #1111 NetPay: 51666.67 yearlySalary: 62000.00
Employee #30495848484 Dept #2222 NetPay: 860.00 HourlyRate: 21.50 Hours: 40.0
Employee #40394833333 Dept #3333 NetPay: 4300.00 Sales: 30000.00
Employee #48584842283 Dept #2232 NetPay: 475.00 HourlyRate: 10.00 Hours: 45.0
Employee #38485849494 Dept #2222 NetPay: 656.25 HourlyRate: 15.00 Hours: 42.5
Employee #34345564433 Dept #1111 NetPay: 46666.67 yearlySalary: 56000.00
Employee #98605050393 Dept #3333 NetPay: 1440.00 Sales: 12000.00
Employee #00033432738 Dept #3333 NetPay: 2100.00 Sales: 17000.00

[Mythios]

Please put your code inside code tages like such:

[code]

//Your code here

[/code]

Otherwise a huge chunk of writing is just a mess to look at :)

[wretch]

Got it! That certainly makes a "small" difference...

[seymore15074]

Not bad. I have only two criticisms (and I'm very picky).

1. The data members are public! That eliminates the data encapsulation. Also, the assignment specified that they should be protected.

2. The dynamically allocated memory is never freed by calling delete.

EDIT: Make that 3 criticisms...

3. If you intended to delete the base class pointer, you need virtual destructors.

[wretch]

Thanks seymore15074. I am working on putting in your changes. I will have to:

1.make data members private.
2.create some setters and/or getters for those data members.
3.make call to delete empPtr after the empPtr->readWageInfo call.
4.create virtual destructors so I can delete base class pointer.

#4 I am reading about right now, it will take me a while to begin understanding virtual destructors, so far what I read is confusing me.

Anyways, thanks again for taking the time to look at my code. Much appreciated.

[seymore15074]

Well, actually, you should make the data members of Employee protected (so that the derived classes can access them directly) and the derived class data members private, if you do not intend to inherit from them later.

Accessor methods would not be required for this (so far), since only the derived class methods use the data.

Glad to help.