This topic is 3178 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

I am trying to program a Workout log, compiling it step by stet to see if there is any problem. As always, there is one. When I build it, it gives me no errors, and I am able to compile it. But during runtime it stops executing and the system crashes I am not sure why. However I did debug it found the problem lies with the vector string I created. The program crashes when it reaches the for loop, more specifically when I call for the vector.size(); here is the relavent code,although ask if you need to see more :

{
system("cls"); //Could not resist

unsigned int i(0),j(0);

workOutRoutine *Temp = new workOutRoutine;
//Used to determine the end of the list to add new node
workOutRoutine *EndList;

//Dummy variable
string dummy;
getline(cin,dummy);

//Get Day Of workout;
cout<<"Enter the Day : ";
getline(cin,Temp->Day);
cout<<endl;
//Get body thats being worked out
cout<<"Which Body Part is Worked out on "<<Temp->Day<<"\t";
getline(cin,Temp->bodyPart);
cout<<endl;

//on the unique body part
//and the number of sets
//each with certain weight
for(i =0; i <= Exercise.size(); i++) //<--- PROBLEM LIES WITHIN "Exercise.size(). While degugging it crashes my program.
{
if(i == Exercise.size()-1)
{
cout<<"\nWould You like to add more sets to your "<<Temp->bodyPart<<" workout (y/n) ";
char opt;
cin >> opt;

//NOT SURE IF THIS PART WORKS THE WAY I WANT IT TO,
//BUT I'LL DEAL WITH THAT AFTER THIS BUG IS FIXED
if(opt == 'y' || opt == 'Y')
{
Exercise.resize(Exercise.size()+1,vector<string>(Exercise.size()+1));
}

else break;

}

cout<<"What type of "<<Temp->bodyPart<<" workout did you do : ";
getline(cin,Exercise[0]); // [0] is the Identifier or type of workout
cout<<"\n"<<endl;

for(j = 1; i< Exercise.size(); j++)
{
cout<<"Enter the weight lifted for set # "<<++j<<"  :  ";
getline(cin,Exercise[j]);
}

}

cout<<"If you did any extra 'workout' the enter keywords with time like basketball(30.00 minutes),running(10.00 minutes)\n";
cout<<"else enter NONE :  ";
getline(cin,Extra);

cout<<"\nWhat do you estimate the total time you took to workout :  ";
getline(cin,TotalTime);

//Check to see if any node has been alocated
if(strt == NULL)
{
strt = Temp;
strt->NextNode = NULL;
return;

}
else
{	//set endlist pointer to start of the list
EndList = strt;

//Move to the end of the list
while(EndList->NextNode != NULL)
EndList = EndList->NextNode;

EndList = Temp; //Add new node
EndList->NextNode = NULL;

}
}


Regards, Tnutty

##### Share on other sites
Are you sure that the pointer you're calling AddNode() on is valid?

##### Share on other sites
Yes, I think so, here is the full code

#include<iostream>#include<string>#include<vector>#include<windows.h>using namespace std;//Min number of Exercie that I will do per Workoutconst unsigned int MIN_EXERCISE = 3;//The number of sets/exercise done //for a particular workoutconst unsigned int MIN_SETS = 4;//Class hold every info about the type of workout, which days it on// which body part, how many sets, how much weight lifted per set, and //the time it toolclass workOutRoutine{public: 	//The day of workout	string Day;	//The body part thats being worked out	string bodyPart;	//The type of exercise begin used	//And how many times being done	//Exercise[x][0] is Exercise identifier	//Exercise[x][y] is the weight lifted	vector<vector<string> > Exercise;	//Any extra exercise done	string Extra;	//The time spent during a particular day of working out	string TotalTime;	workOutRoutine *NextNode;public:	workOutRoutine();	void AddNode();	void PrintInfo();	bool DeleteFirstNode();	bool DeleteLastNode();	bool DeleteCurrentNode();	void MoveRight();	void ModeLeft();	bool SaveToFile();} *strt,*curr;//The constructorworkOutRoutine::workOutRoutine(){	//set strt so that its empty	strt = NULL;	curr = strt;	//Resize vector to the minimun number of exercise that 	//I will do and to the minimum number of sets	Exercise.resize(MIN_EXERCISE,vector<string>(MIN_SETS,"NONE"));	}void workOutRoutine::AddNode(){	system("cls");	unsigned int i(0),j(0);	//Used to add new node	workOutRoutine *Temp = new workOutRoutine;	//Used to determine the end of the list to add new node	workOutRoutine *EndList; 	//Dummy variable 	//getline reads \n	string dummy;	getline(cin,dummy);	//Get Day Of workout;	cout<<"Enter the Day : ";	getline(cin,Temp->Day);	cout<<endl;	//Get body thats being worked out	cout<<"Which Body Part is Worked out on "<<Temp->Day<<"\t";	getline(cin,Temp->bodyPart);	cout<<endl;	//Add the type of workout	//on the unique body part	//and the number of sets	//each with certain weight	for(i =0; i <=Exercise.size(); i++)	{		if(i == Exercise.size()-1)		{			cout<<"\nWould You like to add more sets to your "<<Temp->bodyPart<<" workout (y/n) ";			char opt;			cin >> opt;						if(opt == 'y' || opt == 'Y')			{				cout<<"\nAdding another Workout Element...";				Exercise.resize(Exercise.size()+1,vector<string>(Exercise.size()+1));				cout<<"\nDone adding another Element"<<endl;			}						else break;		}		cout<<"What type of "<<Temp->bodyPart<<" workout did you do : ";		getline(cin,Exercise[0]); // [0] is the Identifier or type of workout		cout<<"\n"<<endl;			for(j = 1; i< Exercise.size(); j++)			{				cout<<"Enter the weight lifted for set # "<<++j<<"  :  ";				getline(cin,Exercise[j]);			}	}	//Get Additional Info	cout<<"If you did any extra 'workout' the enter keywords with time like basketball(30.00 minutes),running(10.00 minutes)\n";	cout<<"else enter NONE :  ";	getline(cin,Extra);	cout<<"\nWhat do you estimate the total time you took to workout :  ";	getline(cin,TotalTime);			//Check to see if any node has been alocated	if(strt == NULL)	{		strt = Temp;		strt->NextNode = NULL;		return;	}	else	{	//set endlist pointer to start of the list		EndList = strt;		//Move to the end of the list		while(EndList->NextNode != NULL)			EndList = EndList->NextNode;		EndList = Temp; //Add new node		EndList->NextNode = NULL;	}}void workOutRoutine::PrintInfo(){	workOutRoutine *Temp = strt;	//Check if any node exist	if(Temp == NULL)	{		cout<<"No routine at this point\n\n";		return;	}	//Print all Info	while(Temp != NULL)	{		cout<<"Day of Week "<<Temp->Day<<endl;		cout<<"Body Part worked out : "<<Temp->bodyPart<<endl;		for(unsigned int i = 0; i<Exercise.size(); i++)		{			//Exercise[0] represents Workout Type/ID			cout<<"Type of Workout #"<<i<<"\t\t"<<Temp->Exercise[0]<<endl;			for(unsigned int j = 1; j< Exercise.size(); j++)			{				//Exercise weight lifted				cout<<"\t  Set #"<<i<<" : "<<Exercise[j]<<endl;			}		}				cout<<endl<<endl;			}}int Options(int opt){	cout<<"1 -- Add Day to workout"<<endl;	cout<<"2 -- Move Node forward"<<endl;	cout<<"3 -- Move Node Backward"<<endl;	cout<<"4 -- Delete current Node"<<endl;	cout<<"5 -- Delete First Node"<<endl;	cout<<"6 -- Delete Last Node"<<endl;	cout<<"7 -- Save Info to File"<<endl;	cout<<"8 -- Load Info to File"<<endl<<endl;	cin >> opt;	return opt;}int main(){	int opt = -1;	//Call the constructor 	workOutRoutine();	do	{		system("cls");		strt->PrintInfo();		cout<<"\n\n"<<endl;				//Get uses input		opt = Options(opt);		switch(opt)		{			case 1:	strt->AddNode();		break;			case 2:			break;			case 3:			break;			case 4:			break;			case 5:			break;			case 6:			break;			case 7:			break;			case 8:			break;		}	}while(opt != 0);}

##### Share on other sites
Where do you initialize strt?

##### Share on other sites
the Ctor
workOutRoutine::workOutRoutine(){	//set strt so that its empty	strt = NULL;	curr = strt;	//Resize vector to the minimun number of exercise that 	//I will do and to the minimum number of sets	Exercise.resize(MIN_EXERCISE,vector<string>(MIN_SETS,"NONE"));	}

##### Share on other sites
You may not realize this, but NULL is not a valid pointer value to call a member function on.

##### Share on other sites
I tried this before with another linked list program and it gave no problem.

If set it to null in main, it gives the same problem.

Here is the error:
Unhandled exception at 0x008132e9 in ExerciseRoutine.exe: 0xC0000005: Access violation reading location 0x0000004c.

##### Share on other sites
The size function is 1..n not 0..n ( use less-than not less-than-equal)

for(i =0; i <= Exercise.size(); i++)

it tries to iterate one step further than the actual size

... that at least for a start.

Its one of the most common errors -- an 'off by one' error

##### Share on other sites
Quote:
 Original post by wodinoneeyeThe size function is 1..n not 0..n ( use less-than not less-than-equal)for(i =0; i <= Exercise.size(); i++)it tries to iterate one step further than the actual size ... that at least for a start.Its one of the most common errors -- an 'off by one' error

I do this to make push_back func work
when (i) reaches vec.Size(), since it starts from 0, its 1 size passed it, so I
call push_back method if I want to add more sets.

if(i == Exercise.size())		{			cout<<"\nWould You like to add more sets to your "<<Temp->bodyPart<<" workout (y/n) ";			char opt;			cin >> opt;						if(opt == 'y' || opt == 'Y')			{				cout<<"\nAdding another Workout Element...";				Exercise.resize(Exercise.size()+1,vector<string>(Exercise.size()+1));				cout<<"\nDone adding another Element"<<endl;			}

##### Share on other sites
Quote:
 Original post by tnuttyIf set it to null in main, it gives the same problem.

Quote:
 Original post by SiCraneYou may not realize this, but NULL is not a valid pointer value to call a member function on.

##### Share on other sites
So how would you try to solve this?

##### Share on other sites
Quote:
 Original post by tnuttySo how would you try to solve this?

Initialise *all* the variables you want to use. You do not need to use globals here. Pointers need to be pointed at something, either via new or the address of a local.

See where you "call the constructor"? You are creating a temporary here. However, you appear to want it to have side effects. This is not a great idea, it is very unclear.

You should be using values, you don't need to use pointers in your program. Use std::list<> as a linked list.

AddNode() should not be doing I/O. Generally, you want to separate I/O from program logic. Also, this would be a lot easier if you separated the container logic from the work out routine part. Again, std::list<> rather than writing your own container.
Quote:
 //The type of exercise begin used//And how many times being done//Exercise[x][0] is Exercise identifier//Exercise[x][y] is the weight liftedvector > Exercise;

You should make a type to represent this information, rather than a raw vector<string>.

Here is a bug:
for(j = 1; i< Exercise.size(); j++){   // ...}

And are you sure you want to increment "j" in this next line?
cout<<"Enter the weight lifted for set # "<<++j<<"  :  ";

##### Share on other sites
I realize that I could do this program in another way, its just for practicing

so in this code :
Here is a bug:for(j = 1; i< Exercise.size(); j++){   // ...}

I am not exactly seeing why the problem exist. I realize that the call
to Exercise.size() is the bug. But using a pointer(pointing to it) causes it
read the memory in a invalid area? Why so?

##### Share on other sites
You generally loop from 0 to size(), not 1 to size(). Besides, your condition uses "i", not "j" as you use in the loop setup and loop increment.

##### Share on other sites
Quote:
 Original post by tnuttyI realize that I could do this program in another way, its just for practicinglinked list.

Practicing writing bad code will only teach you to write bad code.

##### Share on other sites
So what advice would you give me to write better code for this program.
program better?

##### Share on other sites
Quote:
 Original post by tnuttyI realize that I could do this program in another way, its just for practicinglinked list. Please bare with me.

I found implementing my own templated linked list hugely educational and I would recommend it to others. However, what you're doing here is a mess. There is no separation between the container and the rest of your program.

If you want to learn about containers, I think you should write an implementation of the interface provided by the Standard C++ Library. http://www.cplusplus.com/reference/stl/list/

##### Share on other sites
Quote:
 Original post by Cantos However, what you're doing here is a mess. There is no separation between the container and the rest of your program.

I just started to learn linked list from online tutorial, and this was the way
he implemented it. But,since my program is such a catastrophe what guidelines would you provide for implementing a decent linked-list?

##### Share on other sites
Quote:
 But,since my program is such a catastrophe what guidelines would you provide for implementing a decent linked-list?
Start a new program free of all cruft. You are not making anything but a linked list. You are not making a workout program. You are not making pretty text interfaces. Just a linked list implementation.

Then start by getting the data structures right. You need the syntax to represent a node in the linked list, and the syntax to represent the entire linked list itself. This is not a given. Focus on getting this right. Verify it on this forum if you need to do so. Then work on getting on adding algorithms to this structure, one by one.

##### Share on other sites
Quote:
 Original post by tnuttyBut,since my program is such a catastrophe what guidelines would you provide for implementing a decent linked-list?

Well, first off, why do you want to implement one? The things that you need to know in order to do it properly can be learned just as well in many other ways.

##### Share on other sites
Quote:
Original post by Zahlman
Quote:
 Original post by tnuttyBut,since my program is such a catastrophe what guidelines would you provide for implementing a decent linked-list?

Well, first off, why do you want to implement one? The things that you need to know in order to do it properly can be learned just as well in many other ways.

For practice, as other of my recent comment suggests.

You know, I think I picked up some bad habits from online tuts, like from this

##### Share on other sites
Tripod is a site that lets the general public create small webpages. You shouldn't really trust it for quality, for that reason: you need to verify that the author knows what s/he is talking about. And I would guess, without even reading the page, from that colour scheme and background choice that the author of the tutorial in question is not exactly a professional. :)

##### Share on other sites
I will think as I write.

In the first box of code, I cannot find any instance where you actually define and set the variable "Exercise", hence it is a NULL pointer. The size of a NULL pointer is also undefined, so the loop accesses an undefined variable to determine how many times to loop and has no clue.

*****
//on the unique body part
//and the number of sets
//each with certain weight
for(i =0; i <= Exercise.size(); i++) //<--- PROBLEM LIES WITHIN "Exercise.size(). While degugging it crashes my program.
*****

At this point you have "i" and "j" defined, "Temp" is a pointer to a workOutRoutine structure as also is the variable "EndList". You also have a string "dummy", (no I am not calling you a dummy),but no where is any variable mentioned "Exercise" until you access it in the loop. The program does not know if it is a string, integer or whatever. The address to this variable can be any number. It happens that it is near zero, which is protected memory where the operating system resides. That is why the access violation. The location, ie: memory address is 4C, that is a low number in the area of BIOS and other system memory.

I see in the full code that it is defined as an array of strings I think, but I have not used the vector function. I do know that when you setup some arrays, especially with strings it starts out with an undefined address until you first assign a value of some sort. I use a listing program in addition to debugger. It tells you a lot more. If you step through the debugger and look at what value "Exercise" is as it progresses, have the debugger stop just before the loop, check to see what "Exercise" is, the address and the value.

I have also never encountered a program that uses a for loop command with a variable in the loop command that is modified by the loop itself. I have always had the range, 1 to 6 or 1 to 10, set before starting the loop. If I needed to vary the number of iterations, I would use a repeat, or a while loop instead. These commands are designed to be variable, whereas the for loop is usually set before the iterations begin. At least that is the conformity that I stick to.

It looks like you are having the arrays set to 3 and 4, and the string initialized to "NONE" to start off with, if I understand the program. So, it looks like the array is an array of arrays, Exercise[3][4]. I think that the problem is with the size of the array. You really do wish to progress through the exercises, which is 3 and then set the string variable in each of the sets within each of the exercises. Problem? Does size return 12, or does it return 3 when you ask for the size of Exercise? You use the similar statement for each loop, in the "i" portion of the loop, ie: number of exercises and the "j" portion of the loop, the sets. If it returns 12 each time for the whole array, and you want to get 3, then.....

So, it will step through 12 times for each of the loops.

loop 1 = Exercise[0][0] (This is the name of the workout?)
loop 2 = Exercise[0][1] (This is the weight of each set?)
loop 3 = Exercise[0][2]
loop 4 = Exercise[0][3] Oops, Crash, not defined

it even wants to go all the way to:

loop 3 = Exercise[0][11]

I am not sure what the size function will return on the whole set of arrays, whether it returns the number of arrays, or the total number of elements. I have not been programming for a while. I think this is the problem. You need to run the debugger and ask to view the size of the variable Exercise, to see if it is 3, 4 or 12.

In the documentation it states that the size function "Returns the number of elements in the vector container". This would tell me that this number is 12.

Conclusion, set a stop just before the loop and investigate what the size value of the array is. If it is 12, then your loop is going too far.

I hope that I have been clear as mud. :)

Thank you.

##### Share on other sites
Quote:
 Original post by ScharfschuetzeIn the first box of code, I cannot find any instance where you actually define and set the variable "Exercise", hence it is a NULL pointer.

Incorrect: it's a vector of string vectors. See the code in his second post. There's nothing wrong with that - both vectors and strings have default constructors. Besides, pointers need to be dereferenced - but he's using the dot operator instead. So, no pointers there. As others already pointed out, he's probably calling that AddNode function on a bad pointer.

A vector is not a function, bytheway - it's a container class, and part of C++' standard library.