string types not registering?

[eripiomundus]

I'm a noob at this, and I'm learning, well, struggling, to learn headers/classes/etc. I really can't figure out what is wrong with the following code except that 'string' types aren't changing colour (in visual studio 2017) when they aught to. This is only happening in the functions.cpp and Header.h files. When I try it in the main.cpp it changes colour as normal.

I'm trying to make a simple Students class with a constructor that gets passed a string (name of student). Then it assigns a student number and initializes some marks at 0 before asking for user input for the actual marks values. Then it returns a student summary.

Sorry for not knowing how to properly frame what's wrong, but it seems to all hinge around the string declarations and passing strings to class methods and such. Here is the code

main.cpp

#include <iostream>
#include <string>

#include "Header.h"

using namespace std;

int main()
{
	Students s1("John Smith"); 

	s1.inputMarks();
	s1.studentSummary();

	system("pause");
	return 0;
}

Header.h

#include <iostream>
#include <string>

#ifndef Students
#define Students

using namespace std;

class Students {

	public:
		Students(string n); 
		void studentSummary();
		void inputMarks();

	private:
		string sName;
		int sNum;
		static int nextID;
		int sMarks[4];
}; 


#endif 

functions.cpp

#include <iostream>
#include <string>
#include "Header.h"

using namespace std;

int Students::nextID = 10000;

Students::Students(string n) {
	sName = n;
	sNum = nextID++;

	for (int i = 0; i < 4; i++) {
		sMarks[i] = 0;
	}
}

void Students::inputMarks() {

	for (int i = 0; i < 4; i++) {
		cout << "Please enter the marks for assessment " << i + 1 << " > " << flush;
		cin >> sMarks[i];
		cout << endl;
	}
}

void Students::studentSummary() {
	int runningTotal = 0;
	cout << "Student name >	" << sName << endl;
	cout << "Student No >	" << sNum << endl;
	for (int i = 0; i < 4; i++) {
		cout << "Marks for assignment " << i << " > " << sMarks[i] << endl;
		runningTotal += sMarks[i];
	}

	cout << "Total marks > " << runningTotal << " > ";
	if (runningTotal > 80)
		cout << "HD\n";
	else if (runningTotal > 70)
		cout << "D\n";
	else if (runningTotal > 60)
		cout << "C\n";
	else if (runningTotal > 50)
		cout << "P\n";
	else
		cout << "N\n";
}

[jlb]

Does your code compile without errors or warnings?

[Handy Andy]

Hello eripiomundus,

Along with what jlb has asked the errors that you should have received is good information of what to fix.

It took me awhile to figure out the problems because some were not easy to find.

In your ".cpp" files it is best not to use the line using namespace std; as it WILL get you in trouble some day. Even now it is a bit of a problem I think.

As a note in "main" you include the header file "Header.h" before the using statement, so the header file can not make use of the using statement. In the end it is better to learn to qualify what is in the standard name space such as ; "std::cout", "std::cin", "std::endl", and " "std::flush" along with others. Learning this now is easier than trying to learn t all at once later.

Another note: When creating the header file and ".cpp" file for a class it is better to name the files the same as the class. For the header files I like to use the extension ".hpp" so as not to confuse the header file with a C header file using the extension ".h".

In your header file the include guard uses the name "Students". This "#define" overshadows the class name. It is best to use all caps with a "#define". As you will see in my revised code this made a big difference.

Note: for, while loops and all manner of if statements with only one line of code do not need the {} unless you have plans to add to it later.

I revised your code to look like this:

main:

#include <iostream>
#include <string>
#include <limits>

#include "Student.hpp"

int main()
{
	Students students("John Smith");

	students.inputMarks();
	students.studentSummary();

	// The next line may not be needid. If you have to press enter to see the prompt it is not needed.
	std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.
	std::cout << "\n\n Press Enter to continue: ";
	std::cin.get();

	//system("pause");

	return 0;
}

Student.hpp:

#pragma once

#ifndef STUDENTS
#define STUDENTS

class Students
{
public:
	Students(std::string n);
	void studentSummary();
	void inputMarks();

private:
	std::string sName;
	int sNum;
	static int nextID;
	int sMarks[4];
};

#endif // !STUDENTS 

Student.cpp:

#include <iostream>
#include <string>

#include "Student.hpp"

int Students::nextID = 10000;

Students::Students(std::string n)
{
	sName = n;
	sNum = nextID++;

	for (int i = 0; i < 4; i++)
		sMarks[i] = 0;
}

void Students::inputMarks()
{

	for (int i = 0; i < 4; i++)
	{
		std::cout << "Please enter the marks for assessment " << i + 1 << " > " << std::flush;
		std::cin >> sMarks[i];
		std::cout << std::endl;
	}
}

void Students::studentSummary()
{
	int runningTotal = 0;

	std::cout << "Student name >	" << sName << std::endl;
	std::cout << "Student No >	" << sNum << std::endl;

	for (int i = 0; i < 4; i++)
	{
		std::cout << "Marks for assignment " << i << " > " << sMarks[i] << std::endl;
		runningTotal += sMarks[i];
	}

	std::cout << "Total marks > " << runningTotal << " > ";

	if (runningTotal > 80)
		std::cout << "HD\n";
	else if (runningTotal > 70)
		std::cout << "D\n";
	else if (runningTotal > 60)
		std::cout << "C\n";
	else if (runningTotal > 50)
		std::cout << "P\n";
	else
		std::cout << "N\n";
}

In "main" it is best not to use "system" anything. Not only is it not portable it limits the code to Windows only. The four lines I added above the "system" call are a fair replacement. And as the comments say do not forget to include the header file "<limits>".

Also notice how the {} for blocks line up in the same column. Not only does this make the code easier to read, with proper indenting, it makes it easier to find a missing closing }. Although there is nothing wrong with what you did I find that it makes for neater code.

Hope that helps,

Andy

[eripiomundus]

Wow man! Thank you heaps for putting in so much effort! I'm still not certain what I was doing wrong, but your improved code compiles and runs perfectly. I appreciate all the tips as well, and I'll definitely take them on board. One thing I didn't understand - and google didn't help me with either. You said: "This "#define" overshadows the class name.". What does that mean?

It's helpful people like you that inspire the world toward ever better things. Thanks again.

[Handy Andy]

Hello eripiomundus,

Thank you and you are welcome.

Overshadows may be the wrong choice of words here, itis just that I have seen it used many times here and it was the first work that came to mind. How about stomps on or covers up.

The point is that "#define Students" has made use of the variable or name "Students" so when you get to "class Students" it does not work because "Students" has already been used and the compiler does not like it when you try to use the same name twice. That is why for the "# define" capital letters are used. And I would/should have used "STUDENTSHPP" with the "HPP" to denote that it is a header file. So if you had used "StudentsHpp" everything would have been fine. or some examples go to the include files subdirectory, mine is C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include, yours may be diffferent and look at the files. Those with a ".h" extension may be easy to see. Those with no extension may need to be opened individually. At the beginning of the file you can see examples of how the "#define"s are used. The underscores are not necessary in what you write, but I would say choose a form and stay consistent with it. Generally I like to use the file name with the extension all as one word with the file name either being the name of the class or something that describes what is in the file.

The only exception I can think of for using the same variable name more than once is in functions.

An example:

void FunctionUsingCount(int count, ...);

int main()
{
	int count{};

	FunctionUsingCount(count, ...);

	return;
}

void FunctionUsingCount(int count, ...)
{
	// Code using "count" inside function.
}

Crude example. The point is that "count" is defined in the scope of "main". When "main" calls the function the scope of "main" is essentially put on hold while the scope of the function becomes active, so anything defined between the ()s and the {}s of the function now have local scope until the function ends and all disappear returning scope to "main". Now in the function you have the ability to change the name of a variable, but sometimes it is better to use the same variable name that was used to call the function just for clarity in the code.

In the future be careful with your variable name and where they are used.

Different ways of defining a constant:

#define MAXSIZE 10

constexpr std::size_t MAXSIZE{ 10 };  // <--- The newer vision of defining a constant.

const std::size_t MAXSIZE{ 10 };  // <--- The older vision of defining a constant. 

My point here is not to show a redefinition of the variable "MAXSIZE", but how it can be defined. Notice that "MAXSIZE" is in capital letters to show that it is a constant that can not be changed. You can think of "std::size_t" as another name for "unsigned int". As I have been told "size_t" is a hold over from C and more or less built into the IDEs and compilers. The "std::" is not really necessary, but good to learn. "MAXSIZE" is a variable name that is often used to define the size of an array or used as the upper limit of something. The "{ 10 }" is known as a uniform initializer and I would say the easiest way to initialize a variable. A set of empty {}s will initialize a numeric variable to zero or 0.0 for floats and doubles. A "char" will be initialized to "\0". Strings and some other containers are empty to start with and do not need initialized.

Anything else let me know.

Hope that helps,

Andy