Skip to main content
gamedev.net
🔒 Locked

[C++] What's wrong with code?

Started by Utko Aug 19, 2009 at 1:00 PM 17 replies 3k views
Original Post
Utko
Utko
Hey everyone! Could someone please help me out with this... 
int main()
{
 	vector<string> names;
 	vector<double> grades;
	string name; 
 	cout << "Enter your names: ";
 	while (cin >> name) {
		  names.push_back(name);
		  if (name == "q") break;
    }
    int i = 0;
    while (i < names.size())
    {
	 	cout << names + ", enter you grade: ";
	 	cin >> grades;
	 	cout << endl;
	 	i++;
	}
	
    system("PAUSE");
    return 0;
}
this doesn't work, for some reason. It works perfectly up until I enter the grade for the first student, and after I press enter, there's a Windows error and the program just shuts down. I think it's obvious what I'm trying to do here, so why doesn't it work? I can't think of a single reason, apart from maybe some type mismatch error... Please help! I tried using a for loop, but the result is the same.
ArthY303
ArthY303
As _fastcall said ... grades has 0 elements so you have to use push_back() every time you read a grade. You can use an auxiliary variable like this :

double aux;

cin>>aux;

grades.push_back(aux);
MrPickle
MrPickle
I'm pretty sure there's a way to resize the vector, I'm probably wrong though.

It'd mean you wouldn't need the temporary variable and you can just continue as you were.
Here to learn :)
SiCrane
SiCrane
Quote:
Original post by MrPickle
I'm pretty sure there's a way to resize the vector, I'm probably wrong though.

Like the resize() member function?
fastcall22
fastcall22
Quote:
Original post by MrPickle
I'm pretty sure there's a way to resize the vector, I'm probably wrong though.


Nope, you are quite correct:

int i = 0;	grades.resize( names.size() );while (i < names.size()){	cin >> grades;	// Dot dot dot}
Driv3MeFar
Driv3MeFar
Quote:
Original post by MrPickle
I'm pretty sure there's a way to resize the vector


There is.

Edit: Much too slow, I must need more coffee.
MrPickle
MrPickle
Looks like I should have more faith in myself (:
Here to learn :)
Utko
Utko
Wow, thank you guys! You're very very helpful here! And there's so much I have already learned just by doing this seemingly pointless Accelerated C++ exercise!
Shadowwoelf
Shadowwoelf
I found three things that aren't necessarily wrong, but would in the end help you.

1. You aren't checking the value of the name before pushing it.

names.push_back(name);if (name == "q") break;


You should check to see if name==q first if it isn't then you push the name on to the vector.

2. If you think about this as a real world application with hundreds of different names/grades you may want to just be able to input both values at the same time. e.g. get a name then get a value for that name. (You could use a map for this but two vectors will work just as well).


3. You may want to check if q is upper case or lower case.

if (name == "q"||name=="Q") break;


(For future reference there is a string function that can compare 2 strings for you. http://www.cplusplus.com/reference/string/string/compare/)

Also for your last while loop it would be the same as using a for loop.
Utko
Utko
Quote:
Original post by dmatter
Have you tried entering a name like "John Doe" into this?


Nope, because that's not gonna work. I'll need to have separate variables for first and last names and then batch-concentrate them somehow.
fastcall22
fastcall22
dmatter reveals an important point: cin >> word only extracts one word from a stream. If you want to get more than that, you can use getline.

//// Answer below://while ( 1 ){	string line;	getline( cin, line );	if ( cin.fail() || line == "q" )		break;	names.push_back( line );}
Captain P
Captain P
Another thing I'd like to point out: every student seems to have a grade attached. Why not group these in a struct or class, and create a vector of those?
struct Student{   std::string name;   double grades;};std::vector<Student> students;students.push_back(Student());students[0].name; // Do stuff with itstudents[0].grades; // Do stuff with it

This ensures that every student has a name and a grade, always. You can add a constructor to set default values for the name and grades, and other member functions to make operations easier.

Note that, whenever you're using a counter, such as with your grades entering code, it's more common to use a for loop:
for(int i = 0; i < names.size(); i++){   // Do stuff}

'i' only exists within the for loop, once the loop has ended 'i' is out of scope. Consider the following code:
int i = 0;while(i < names.size()){   // Do stuff   i++;}while(i < grades.size()){   // Do other stuff   i++;}

Oops... 'i' still existed, but was never reset. A subtle mistake that's easy to overlook. Well, it's a bit of personal preference, but I think a for loop can be a little bit 'safer' in this regard.
c_olin
c_olin
I would suggest using iterators when traversing a vector. Because it prevents you from accessing data outside of the bounds of the vector; which is what your issue was...

for (vector<string>::iterator it = names.begin(); it != names.end(); ++it) {    double aux;    cout << *it << ", enter your grade: ";    cin >> aux;    grades.push_back(aux);}
Utko
Utko
One last question: could I have just assigned something (anything) to grades[0] before entering the loop, and then used my original code? Would that have eliminated the problem?
ArthY303
ArthY303
In your original code ? No ... grades[0] is the first element in the vector and it doesn't exist until you push it back or until you allocate memory for it by using one of vector's methods.You have to allocate memory for each element prior to using them.By using the resize() method you allocate memory for more than one element.(rezise() is good when you know how many elements the vector has which is your case)
And it wouldn't have solved your problem.
c_olin
c_olin
No... you can't really add to a vector without using push_back().

It seems like your intention is to have a grade for every name. This would be a solution:

vector<pair<string, double> > students;while (whatever) {    string name;    double grade;    cout << "Enter name: ";    cin >> name;       cout << endl << "Enter grade: ";    cin >> grade;    students.push_back(pair(name, grade));}


Your method would work, but you are trying to use a vector like a C array. It will work, but it will be easier for you to write buggy code (as you have found out first hand).
bibalasvegas
bibalasvegas
To the people saying use getline/iterators/student struct etc he's following accelerated c++ which does cover all of those in the same example but later. To the op I think you've sorted out your problem but keep following the book because it is very good (imo).

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.