Variable Conversions

[TheIdeasMan]

Just an some ideas:

Don't have line 7. Put std:: before each std thing instead, there is heaps written about why this is so.

Edit: CodeGoggles already mentioned this:
With constructors, investigate using initialiser lists rather than assignment. List the member variables in the same order as what they are declared in the class.

With member variables name them with a leading m_

Avoid Hungarian notation, although I personally limit it to Class names (CMyClass) and pointers (pMySmartPointer).

Don't be afraid to have longer variable names if it makes them meaningful e.g. AnnualSalary instead of anSal and FirstName instead of fname. Same thing for function names.

Consider putting class function definitions in a separate .cpp file, while the class declaration goes in a header file (.h)

Limit the number lines of code in a function to 20 or 40 say, if a member function is too long, create some private functions to do some of the work.

I personally hate dislike no hate constructs like this:

while ((gen != ("female")) && (gen != ("male")) && (gen != ("Female")) && (gen != ("Male")))

Keep it simple, don't try to compare every combination of a string. Just use a single char, make use of the toupper or tolower functions to halve the logic. If you do have a bunch of things to choose from (and they are constant values) then use a switch with a default case. Otherwise, use an if, else if, else chain .

There you go some stuff to think about

[CodeGoggles]

@TheIdeasMan - Some very good points, thanks for the input and here I thought I was alone
in this thread. Cheers man

[sierranm]

Thank you both! I do truly appreciate the help!

How do you use toupper and tolower with string?

Also, I didn't even think about header files! That would make this much easier!

I really don't want to go through and put std in front of everything right now, but I'll make it a habit, same with the variable names. For now I was just keeping everything the same as what I have on paper so I don't confuse myself any further.

Also, CodeGoggles, I took your advice about just returning the function names and using other variables so they're at least being used. I'll post what I have now, but I'll work on trying to create some header files.

I'm sorry if you feel my code is punishment of some sort. I'm trying to grasp at what you say but a lot of this is very new to me.

I'm now dabbling with stringstream...which has given me 45 error messages of the same variety.

#include <iostream>
#include <iomanip>
#include <string>
#include <sstream>
#include <stdexcept>
stringstream ss;
using namespace std;
void displayDivider(int &title, int &i);
//Declaration
const char FileName[] = "EmployeeRecords.txt";

//Class
class Employee
{
private:
	string lname, fname, stemp;
	char gender;
	int dependents, itemp;
	double anSal, wkSal, dtemp;
	//for divider:
	int title;

public:
	// Constructors
	int i = 1;
	Employee()
	{
		fname = "Not Given";
		lname = "Not Given";
		gender = 'U';
		dependents = 0;
		anSal = 20000;
	}
	Employee(string f, string l, char g, int d, double s)
	{

	}
	double Employee::calcPay();
	void Employee::display();
	string Employee::getFirst();
	string Employee::setFirst(string fname);
	string Employee::getLast();
	string Employee::setLast(string lname);
	char Employee::getGen();
	char Employee::setGen(char gender);
	int Employee::getDep();
	int Employee::setDep(int dependent);
	double Employee::getSal();
	double Employee::setSal(double anSal);
	void displayDivider(int &title, int &i);
};
//Divider
void displayDivider(int &title, int &i)
{
	if (title == 1)
	{
		cout << "\n--------------------- Employee " << i << " ---------------------\n";
		i++;
		return;
	}
	else if (title == 0)
	{
		cout << "------------------------------------------------------\n";
	}//end ifs
}//end dividers
//get information:
string Employee::getFirst()
{
	title = 1;
	displayDivider(title, i);
	//input
	cout << "\nEnter First Name:\t";
	ss >> stemp;
	Employee::setFirst(fname);
	return fname;
}
string Employee::getLast()
{
	cout << "\nEnter Last Name:\t";
	ss.clear();
	ss >> stemp;
	Employee::setLast(lname);
	return lname;
}
char Employee::getGen()
{
	cout << "\nEnter Gender:\t\t";
	ss.clear();
	ss >> stemp;
	//check for errors:
	while ((stemp != ("female")) && (stemp != ("male")) && (stemp != ("Female")) && (stemp != ("Male")))
	{
		cout << "Error: Gender must be Male or Female.\n";
		cout << "Enter Gender:\t\t";
		ss.clear();
		ss >> stemp;
	}
	Employee::setGen(gender);

	return gender;
}
int Employee::getDep()
{
	cout << "\nEnter Dependents:\t";
	ss.clear();
	ss >> itemp;
	Employee::setDep(dependents);
	return dependents;
}
double Employee::getSal()
{
	cout << "\nEnter Last Name:\t";
	ss.clear();
	ss >> dtemp;
	Employee::setSal(anSal);
	return anSal;
	title = 0;
	displayDivider(title, i);
}

//set values
string Employee::setFirst(string fname)
{
	fname = stemp;
	return fname;
}
string Employee::setLast(string lname)
{
	lname = stemp;
	return lname;
}
char Employee::setGen(char gender)
{
	if (stemp[0] == 'F' || stemp[0] == 'f')
	{
		gender = 'F';
		return gender;
	}
	else
	{
		gender = 'M';
		return gender;
	}
}
int Employee::setDep(int dependents)
{
	dependents = itemp;
	return dependents;
}
double Employee::setSal(double anSal)
{
	anSal = dtemp;
	return anSal;
}

//ouput
double Employee::calcPay()
{
	wkSal = anSal / 52;
	return wkSal;
}
void Employee::display()
{
	title = 0;
	displayDivider(title, i);
	cout << "Name:\t" << fname << " " << lname << endl;
	cout << "Gender:\t" << gender << endl;
	cout << "Dependents:\t" << dependents << endl;
	cout << "Annual Salary:\t" << anSal << endl;
	Employee::calcPay();
	cout << setprecision(2) << "Weekly Salary:\t" << wkSal << endl;
	displayDivider(title, i);
}

//main
int main(int &title, int &i)
{
	cout << "Assignment:\tLAB2\nDeveloper:\tSierra McGivney\nDate Written:\t05/16/2014\nPurpose:\tInput and output employee information.\n\n";
	Employee one("", "", 'U', 0, 20000);
	Employee two("Sierra", "McGivney", 'F', 0, 65000);//this should be what is displayed for two
	one.getFirst();
	one.getLast();
	one.getGen();
	one.getDep();
	one.getSal();
	cout << "\n\nEmployee Information:\n";
	one.display();
	title = 1;
	displayDivider(title, i);
	two.display();

	return 0;
} //end main

[CodeGoggles]

I'm sorry if you feel my code is punishment of some sort. I'm trying to grasp at what you say but a lot of this is very new to me.

Don't be, all part of learning.

//set values
string Employee::setFirst(string fname)
{
	fname = stemp;
	return fname;
}
string Employee::setLast(string lname)
{
	lname = stemp;
	return lname;
}

I'm not sure you grasped my point please review my previous post. stemp is not required at all. You are still bringing in a string called fname which is the same as the class member name.

//set values
string Employee::setFirst(string stemp)
{
	return stemp;
}
string Employee::setLast(string stemp)
{
	return stemp;
}

This is fixed but you will also need to remove the stemp variable from your class declaration.

And did you grasp the array example I provided?

[sierranm]

Thank you. :)

I poked around with the array, but I worry for the conversion of the char, int, and double characters, and I do only need two, five was overachieving...which I probably won't have time to do.

I'll take another look at what you mentioned if that's the case.

[CodeGoggles]

but I worry for the conversion of the char, int, and double characters

Why?

[CodeGoggles]

How do you use toupper and tolower with string?

http://www.cplusplus.com/reference/cctype/toupper/

[TheIdeasMan]

@sierranm

Consider these points:

1. Set functions are only needed if a member variable needs to be changed after it has been created. An initialiser list initialises member variables as the object is being created - technically before the code in the body of the constructor is executed. You probably don't need any set functions !! If you do actually need them, then they should have code to check that the operation is authorised - we don't want a trivial function like this:

sierra.SetBankBalance(-1000000.00); // instant $1million mortgage!!

There is also no need to call a set function to set a member variable - class functions have direct access to member variables. All this is negated by using the initialiser list.

2. get (accessor) functions return the value of a member variable, not get input from the user. They should be marked const as they don't change the value of any of the member variables. But seen as you have the display function, you won't need any get functions either!!

3. Get input from the user, validate it, then create the object - we want to create a valid object. This all happens outside the class, and not inside the constructor.

4. Avoid using global variables, the filename & stringstreams could exist in main() function scope say.

Changing variable names isn't hard - use find & replace in your IDE or editor. Hopefully your IDE has some refactoring functions which change the names of things everywhere not just in he file you are in. Similarly with placing std:: in the appropriate places, you only have cout, cin, string, endl. If you miss any the compiler will tell you. With std::endl, I only use it occasionally because it flushes the buffer, say at the end of a for loop or function, otherwise use the \n character.

How do you use toupper and tolower with string?

std::toupper works with char, and there is another C++ version with a locale, rather than the C version:

http://www.cplusplus.com/reference/locale/toupper/

More stuff to think about ..... :+D

[sierranm]

@TheIdeasMan

Thank you for your tips, and I thank you both for those links. It gave me enough of a go ahead on how to go about using the toupper. Unfortunately, my stringstream is doing very poorly. Once my code gets to dependents now I get stuck! The Gender does translate though, which is exciting! Just not sure what to do with the ss.

@CodeGoggles

If I just entered it all in as an array with 25 variables, I'd have to manually set 3,4,5,8,9,10,13,14,15,18,19,20...etc. to convert. I imagine it would get confusing. Would it not?

Code Update:

#include <iostream>
#include <iomanip>
#include <ctype.h>
#include <string>
#include <sstream>
#include <stdexcept>

using namespace std;

stringstream ss;

//Class
class Employee
{
private:
	string lname, fname, stemp;
	char gender;
	int dependents, itemp;
	double anSal, wkSal, dtemp;
	//for divider:
	int title;

public:
	// Constructors
	int i = 1;
	Employee()
	{
		fname = "Not Given";
		lname = "Not Given";
		gender = 'U';
		dependents = 0;
		anSal = 20000;
	}
	Employee(string f, string l, char g, int d, double s)
	{
		getFirst();
		getLast();
		getGen();
	}
	double Employee::calcPay();
	void Employee::display();
	string Employee::getFirst();
	string Employee::setFirst();
	string Employee::getLast();
	string Employee::setLast();
	char Employee::getGen();
	char Employee::setGen();
	int Employee::getDep();
	int Employee::setDep();
	double Employee::getSal();
	double Employee::setSal();
};
//Divider
void displayDivider(int &title, int &i)
{
	if (title == 2)
	{
		i = 2;
		cout << "\n--------------------- Employee " << i << " ---------------------\n";
		return;
	}
	if (title == 1)
	{
		i = 1;
		cout << "\n--------------------- Employee " << i << " ---------------------\n";
		return;
	}
	else if (title == 0)
	{
		cout << "------------------------------------------------------\n";
	}//end ifs
}//end dividers
//set information
string Employee::setFirst()
{
	title = 1;
	displayDivider(title, i);
	//input
	cout << "\nEnter First Name:\t";
	cin >> fname;
	Employee::getFirst();
	return fname;
}
string Employee::setLast()
{
	cout << "\nEnter Last Name:\t";
	cin >> lname;
	Employee::getLast();
	return lname;
}
char Employee::setGen()
{
	cout << "\nEnter Gender:\t\t";
	cin >> stemp;
	while ((stemp != "Female") && (stemp != "Male") && (stemp != "female") && (stemp != "male"))
	{
		cout << "Error: Gender must be Male or Female.\n";
		cout << "Enter Gender:\t\t";
		cin >> stemp;
	}//end while
	Employee::getGen();
	return gender;
}
int Employee::setDep()
{
	cout << "\nEnter Dependents:\t";
	ss.clear();
	ss << stemp;
	ss >> dependents;
	Employee::getDep();
	return dependents;
}
double Employee::setSal()
{
	cout << "\nEnter Last Name:\t";
	ss.clear();
	ss << stemp;
	ss >> anSal;
	Employee::getSal();
	title = 0;
	displayDivider(title, i);
	return anSal;
}

//get values
string Employee::getFirst()
{
	return fname;
}
string Employee::getLast()
{
	return lname;
}
char Employee::getGen()
{
	char c = stemp[0];
	c = toupper(c);
	gender = c;
	return gender;
}
int Employee::getDep()
{
	return dependents;
}
double Employee::getSal()
{
	return anSal;
}

//ouput
double Employee::calcPay()
{
	wkSal = anSal / 52;
	return wkSal;
}
void Employee::display()
{
	title = 0;
	displayDivider(title, i);
	cout << "Name:\t" << fname << " " << lname << endl;
	cout << "Gender:\t" << gender << endl;
	cout << "Dependents:\t" << dependents << endl;
	cout << "Annual Salary:\t" << anSal << endl;
	Employee::calcPay();
	cout << setprecision(2) << "Weekly Salary:\t" << wkSal << endl;
	displayDivider(title, i);
}

//main
int main(int &title, int &i)
{
	cout << "Assignment:\tLAB2\nDeveloper:\tSierra McGivney\nDate Written:\t05/16/2014\nPurpose:\tInput and output employee information.\n\n";
	Employee one("", "", 'U', 0, 20000);
	one.setFirst();
	one.setLast();
	one.setGen();
	one.setDep();
	one.setSal();
	cout << "\n\nEmployee Information:\n";
	one.display();
	title = 1;
	displayDivider(title, i);
	Employee two("Sierra", "McGivney", 'F', 0, 65000);//this should be what is displayed for two
	two.display();

	return 0;
} //end main

[Yay295]

If you want to make a std::string uppercase or lowercase, you could use this header:
http://www.mediafire.com/view/2etl2eez309c9l8/string+

[TheIdeasMan]

Hi Sierra,

Just wondering, could you read through my posts with some attention to the detail of what I said ? There are a bunch of things you are doing that don't really make sense: Like calling another member functions to access a member variable while already in a member function. Member functions have direct access to member variables, so there is no need for this.

Here is a challenge for you: Reduce your code by at least half ! :D Remember what I said about the need for get / set functions. And what get / set functions do.

Your get functions could be part of a class that gets input & validates it, or functions defined in a file along with main()

Also line 173: This could be achieved by calling the default constructor.

With the toupper, I think you missed the point with that. It should used when obtaining the input, so as to make the validation easier. So don't ask for a string, then only use the first char. Just ask for a char to start with.

One final thing: Your main is odd. Normally it looks like this:

int main() // sufficient for this project

or:

int main (int argc, char* argv[]) // when you want to process cmd line arguments

More thinking .... 8+)

[sierranm]

Oh, I'm very sorry. I haven't checked back to this post until now.

I've been altering this code quite a bit within the last two weeks and I do realize it's extremely convoluted and some of it is very pointless. I'm actually intending to, once my teacher is done having us add to the code, find a way to make it much smaller. It drives me crazy that I have to scroll through all of that code and make a hunt out of it all.

Here is what I've done with the get/set functions:
Note: I've made some new additions and changes to some of this.

//Set: Employee
string Employee::setFirst(string fname)
{
	cout << "\nEnter First Name:\t\t\t";
	cin >> stemp;
	return stemp;
}
string Employee::setLast(string lname)
{
	cout << "\nEnter Last Name:\t\t\t";
	cin >> stemp;
	return stemp;
}
string Employee::setGen(char gender)
{
	cout << "\nEnter Gender:\t\t\t\t";
	cin >> stemp;
	while ((stemp != "Female") && (stemp != "Male") && (stemp != "female") && (stemp != "male"))
	{
		cout << "Error: Gender must be Male or Female.\n";
		cout << "Enter Gender:\t\t";
		cin >> stemp;
	}//end while
	return stemp;
}
string Employee::setDep(int dependents)
{
	cout << "\nEnter Dependents:\t\t\t";
	cin >> stemp;
	return stemp;
}
string Employee::setSal(double anSal)
{
	cout << "\nEnter Annual Salary:\t\t\t";
	cin >> stemp;
	title = 0;
	displayDivider(title, empCount);
	return stemp;
}

//Set: Benefit
string Benefits::setIns(string provider)
{
	cout << "\nEnter Insurance Provider:\t\t";
	cin >> stemp;
	return stemp;
}
string Benefits::setLife(double lifeIns)
{
	cout << "\nEnter Life Insurance:\t\t\t";
	cin >> stemp;
	return stemp;
}
string Benefits::setDays(int vacationDays)
{
	cout << "\nEnter Vacation Days:\t\t\t";
	cin >> stemp;
	return stemp;
}

//Get: Employee
string Employee::getFirst()
{
	fname = stemp;
	return fname;
}
string Employee::getLast()
{
	lname = stemp; //the reason I use stemp is because I had errors without it.
	return lname;
}
char Employee::getGen()
{
	char c = stemp[0];
	c = toupper(c);
	gender = c;
	return gender;
}
int Employee::getDep()
{
	int value = 0;
	for (int i = 0; i < stemp.length(); i++)
	{
		value *= 10;
		value += stemp[i] % 48;
	}
	dependents = value;
	return dependents;
}
double Employee::getSal()
{
	anSal = stod(stemp);
	return anSal;
}

//Get: Count
void Employee::getCount()
{
	title = 3;
	displayDivider(title, empCount);
	cout << "\nTotal number of employees:\t\t" << empCount << "\n" << endl;
	title = 0;
	displayDivider(title, empCount);
}

//Get: Benefit
string Employee::getIns()
{
	provider = stemp;
	return provider;
}
double Employee::getLife()
{
	lifeIns = stod(stemp);
	return lifeIns;
}
int Employee::getDays()
{
	int value = 0;
	for (int i = 0; i < stemp.length(); i++)
	{
		value *= 10;
		value += stemp[i] % 48;
	}
	vacationDays = value;
	return vacationDays;
}

Unfortunately, I'm required to have a default constructor that returns the values as listed the way the are. line 173 is supposed to be used to set & get the functions.

As for toupper, I have figured out the right way to use it(mostly anyway).
example:

#include <iostream>
#include <cctype>
#include <cstring>
using namespace std;

const int cap = 25;

int main()
{
	char *name;
	int size = 0;

	name = new char[cap];
	cout << "Enter name: ";
	cin >> name;
	size = strlen(name);//counts size of name;

	cout << "Your name is ";
	for (int i = 0; i < size; i++)
	{
		
		name[i] = toupper(name[i]);
		cout << name[i]; //displays letters one by one
	}
	//endline for sepearation of text
	cout << endl;

	delete[]name;
	name = NULL;
	return 0;
}

[CodeGoggles]

I notice you are bringing in information into a lot of functions but you are not actually doing anything with it.