Skip to main content
GameDev.net gamedev.net
🔒 Locked

Runtime error.

Started by rs_man Dec 9, 2009 at 12:41 PM 21 replies 2.5k views
Original Post
rs_man
rs_man
Hey, guys this is my first post here and i want to ask something. Here's the code:

#ifndef LIST_H
#define LIST_H

template <class CType>
class List

{
public:

	List()
	{
	count=0;
	}
//-------------------
	int AddToList(CType item)
	{

		if (count==0)
	{
		ListMain= new CType;
		ListMain[count]=item;
		count++;
	}

	else if(count==1)
	{
		SecondList=new CType;
		SecondList[0]=ListMain[0];
		delete ListMain;
		ListMain=new CType[count+1];
		ListMain[0]=SecondList[0];
		ListMain[1]=item;
		delete SecondList;
		count++;
	}
	else 
	{
	SecondList= new CType[count];

	for (int i=0; i<count; i++)
	{
		SecondList=ListMain;
	}

		delete [] ListMain;

	count++;
	ListMain= new CType[count];

	for (int i=0; i<count-1; i++)
	{
		ListMain=SecondList;
	}
	ListMain[count-1]=item;

		delete [] SecondList;
	}
	return count-1;
	}
//-------------
	void PrintValues()
	{
	for(int i=0; i<count; i++)
		cout<<ListMain<<endl;
	}
//--------------
	CType GetItem(int index)
	{
		return ListMain[index];   <-------------------- my program breaks here.
	}
//--------------
	int GetItemsAmount()
	{
	return count;
	}
	
//----------------
private:
	CType *ListMain;
	CType *SecondList;
	int count;

};

	#endif




My program crashes and I don't know why. I marked the line in which program crashes. Thanks! [Edited by - Zahlman on December 20, 2009 8:08:19 PM]
fastcall22
fastcall22
The most immediate problem could be that you're accessing outside the array. However, I cannot know this because you haven't posted how you're using your class. I'm also assuming that you're doing this for educational purposes. Aside from some redundancy code, the code looks okay. It might be how you're using though

template <class CType>class List {public:	// Constructors should use initializer lists	List() : count(0), // same as count = 0;		MainArray(0) // same as MainArray = 0;	{		// Make sure all members (especially pointers!) are initialized	}			// Pass by const-reference to avoid a copy here	int AddToList( const CType& item) {		CType* SecondList = 0;				if ( count == 0 ) { // If the list is empty			ListMain= new CType; // Allocate a new CType (not an array)			ListMain[count]=item; // Assign the new CVType to be item 			// Note:  ListMain is not an array, and shouldn't be accessed as such			// *ListMain = item;  // <-- more appropriate			count++; // Increment count						// Everything is ok here, except that if you call 			// delete[] on ListMain the universe will subtly			// explode.		} else if ( count == 1 ) { // If the list has exactly one element			SecondList=new CType; // Create new array			SecondList[0]=ListMain[0]; // Copy the old element into the new array			delete ListMain; // Return the old memory to the system			ListMain=new CType[count+1]; // Allocate another array??			ListMain[0]=SecondList[0]; // Copy to the new array??			ListMain[1]=item; // Add the next item			delete SecondList; // Return the memory for the temporary array to the system			count++; // Increment count						// Everything is ok here, except a bit redundant:			// CType* tempArray = new CType[count+1]; // Allocate the new array			// tempArray[0] = *MainList; // MainList, when count == 1, is not an array			// tempArray[1] = item; // Add item to the new array			// delete MainList; // Return the memory for the old array to the system			// MainList = tempArray; // MainList now points to the new array		} else { // And if the list has any other number of elements			// Allocate a temporary array:			SecondList= new CType[count];			// Copy elements in ListMain to SecondList			for (int i=0; i<count; i++)				SecondList=ListMain;			delete [] ListMain;			count++; // Increment count			ListMain= new CType[count]; // Allocate a new array			// Copy the temporary array to the new array			for (int i=0; i<count-1; i++)				ListMain=SecondList;						// Append item to the new array			ListMain[count-1]=item;			// Free the secondary list			delete [] SecondList;			// Everything is ok here, except a bit redundant:			// CType* tempArray = new CType[count+1]; // Allocate a new array with one extra element			// for ( int i = 0; i < count; ++i ) // Copy the old elements to the new array			// 	tempArray = MainArray;			// tempArray[count] = item; // Append item to the end of the new array						// delete[] MainArray; // Delete the old array			// MainArray = tempArray; // MainArray now points to the new memory			// ++count;		}				return count-1; // Return the new number of elements in the array	}	// Use const-correctness:	void PrintValues() const {		for ( int i=0; i<count; i++ )			cout << ListMain << endl;	}	// Be const-correct:	const CType& GetItem(int index) const {		return ListMain[index];	}		int GetItemsAmount() const {		return count;	}		// Free resources on destructor	~List() {		if ( count <= 1 )			delete ListMain; // It's ok to delete 0!		else			delete[] ListMain; // It's ok to delete[] 0!	}//----------------private:	CType *ListMain;	// CType *SecondList; // Not needed	int count;};


It basically collapses to:
template<class CType>class List {public:	List()	: count( 0 )		, array( 0 ) 	{	}	// Rule of three:	List( const List<CType>& secondList ) {		count = secondList.count;		array = new CType[ secondList.count ];		for ( int i = 0; i < secondList.count; ++i )			array = secondList.array;	}	List<CType>& operator = ( const List<CType>& secondList ) {		if ( this != &secondList ) {			delete[] array;			array = new CType[ secondList.count ];			for ( int i = 0; i < secondList.count; ++i )				array = secondList.array;			count = secondList.count;		}		return *this;	}		~List() {		delete[] array;	}		int AddToList( const CType& item ) {		CType* newArray = new CType[ count + 1 ];		for ( unsigned i = 0; i < count; ++i )			newArray = array;				newArray[count] = item;				delete[] array;		array = newArray;				++count;		return count;	}		const CType& GetItem( unsigned idx ) const {		return array[idx];	}		CType& GetItem( unsigned idx ) {		return array[idx];	}		unsigned GetItemsAmount() const {		return count;	}	private:	unsigned 	count;	CType*		array;};


[Edited by - _fastcall on December 10, 2009 12:33:47 PM]
rs_man
rs_man
Thanks. This is the other part of code where i call GetItem function, maybe that helps?
#include <iostream>#include <fstream>#include <string>#include <sstream>#include "List.h"using namespace std;struct Face{	int Vertex[3];	int Texture[3];	int Normal[3];};struct Point{	double x, y, z;};struct Group{string UseMtl;List <Face> Faces;List <Point> Verteces;List <Point> Textures;List <Point> Normals;};class Model{public:	int CreateModel(char* FileName);	void DestroyModel(int index);	void PrintOutFile();private:	Point GetCoord(string str);	Face GetFace(string str);	string GetName(string str);	string MtlLib;	List <Group> group;};Point Model::GetCoord(string str){	Point p;      istringstream ss(str);      string typ;      ss >> typ >> p.x >> p.y >> p.z;	  return p;}Face Model::GetFace(string str) {    Face new_face;    istringstream ss(str);    ss.get(); // skip the f    ss.get(); // skip the space	    string vertice_data;    int vertice_count = 0;    while (getline(ss, vertice_data, ' '))    {        istringstream vertice_data_stream(vertice_data);        string tmp;        // we have a vertice ?        if (getline(vertice_data_stream, tmp, '/'))            new_face.Vertex[vertice_count] = atoi(tmp.c_str());        // we have a texture ?        if (getline(vertice_data_stream, tmp, '/'))            new_face.Texture[vertice_count] = atoi(tmp.c_str());        else            new_face.Texture[vertice_count] = 0;        // we have a normal ?        if (getline(vertice_data_stream, tmp, '/'))            new_face.Normal[vertice_count] = atoi(tmp.c_str());        else            new_face.Normal[vertice_count] = 0;        // is the fact that atoi('') -> 0 an abuse of the system?        vertice_count++;    }	    return new_face;	}string Model::GetName(string str){	string Name;	int pos=str.find(" ");	Name=str.substr(pos+1, str.length());	return Name;}int Model::CreateModel(char* FileName){string line;int Group=0;  ifstream f (FileName);  if (f.is_open())  {	  while (!f.eof())	  {		find:		getline(f, line); 	  if(line.find("#")!=string::npos)		  goto find;	  else if(line.find("mtllib")!=string::npos)	  {		  MtlLib=GetName(line);	  }	  	  else if(line.find("v ")!=string::npos)	  {		  group.GetItem(Group).Verteces.AddToList(GetCoord(line));	  }	  else  if(line.find("vn")!=string::npos)	  {			group.GetItem(Group).Normals.AddToList(GetCoord(line));	  }	  else if(line.find("vt")!=string::npos)	  {		  group.GetItem(Group).Textures.AddToList(GetCoord(line));	  }	  else  if(line.find("usemtl")!=string::npos)	  {			group.GetItem(Group).UseMtl=GetName(line);	  }	  else  if(line.find("f")!=string::npos)	  {		  group.GetItem(Group).Faces.AddToList(GetFace(line));		  	  }	  }    f.close();  }  else	  cout<<"could not open file";  return 1;}void Model::PrintOutFile(){Face f;Point v;Point vt;Point vn;}int main(){ 	Model mdl;	mdl.CreateModel("Box.obj");	mdl.PrintOutFile();  system("pause");  return 0;}


[Edited by - Zahlman on December 20, 2009 8:25:50 PM]
rs_man
rs_man
I want to make it for educational purposes, not to use what others have made.
fastcall22
fastcall22
In that case then...

group.GetItem(Group).Verteces.AddToList(GetCoord(line));

This doesn't do what you think it does. Since GetItem returns by value, when you call GetItem(Group), you're returning a copy of the element in group whose index is Group. And by copy, I mean that this copy and the element in group have their ListMain point to the same memory, but each have their own copy of count. You then proceed to call AddToList on the copy of the element, which invalidates the old array for both copies, but only assigns the new memory for the array to the copy (which it shouldn't), and only increments count on the copy.

GetItem needs to return by reference (and const-reference, since subscript operators come in pairs)

const CType& GetItem(int index) const{	return ListMain[index];}CType& GetItem( int index ){	return ListMain[index];}


[Edited by - _fastcall on December 10, 2009 12:10:31 PM]
rs_man
rs_man
Thank you very much for helping me with my code problem. But program an writing crashes in other line:

int AddToList( const CType& item ) {
CType* newArray = new CType[ count + 1 ];
for ( unsigned i = 0; i < count; ++i ) <---- here
newArray = array;
whats the problem?
And sorry for asking dumb questions, am starting to get a feeling that the program am writing is too complicated for me yet.

[Edited by - rs_man on December 12, 2009 3:22:48 AM]
AverageJoeSSU
AverageJoeSSU
have your tried debugging with print statements (cout)?

if you are know how to use a debugger add a breakpoint (im guessing you dont).

put print statements (cout) for the size of your array in the spots that you are having trouble and see if the index your are looking for actually exists.

also cout "count" in your for loop to make sure its the right count.
------------------------------ redwoodpixel.com
rs_man
rs_man
yes I know how to use debugger, and it says that expression cannot be evaluated ( am talking about count, but I dont know why is that).
Windryder
Windryder
Quote:
Original post by rs_man
And sorry for asking dumb questions, am starting to get a feeling that the program am writing is too complicated for yet.


Which is why _fastcall suggested you use std::vector instead. I understand that you are doing this for educational purposes - however, until you are confident enough with C++ to get this class working on your own, writing an actual game is probably a better idea than writing a substandard implementation of the container classes and then building your game on top of that. As you work with the standard library and C++ in general you will get a deeper understanding of the language so that you can, eventually and if you still want to, take on the task of developing a new set of container classes.

rs_man
rs_man
Probably you are right. I'll give myself 15minutes to find what is wrong in my code and if I can't find anything I'm moving to std::vector.

[Edited by - rs_man on December 12, 2009 4:44:28 AM]
rs_man
rs_man
Hello again, I have moved to std::vector. This is what I have done:
instead of this line:
group.GetItem(Group).Verteces.AddToList(GetCoord(line));
I used this one:
iter=group.begin();
for(int a=0; a {
iter++;
}
*iter->Verteces.push_back(GetCoord(line));

and i get an error:
error C2100: illegal indirection.
What is wrong?

[Edited by - rs_man on December 12, 2009 11:39:00 AM]
nobodynews
nobodynews
Quote:
Original post by rs_man
Hello again, I have moved to std::vector. This is what I have done:
instead of this line:
group.GetItem(Group).Verteces.AddToList(GetCoord(line));
I used this one:
iter=group.begin();
for(int a=0; a {
iter++;
}
*iter->Verteces.push_back(GetCoord(line));

and i get an error:
error C2100: illegal indirection.
What is wrong?


I haven't been following this thread, but there's a chance *iter-> should be (*iter)->

edit: or another possibility is you need (*iter). I'm not sure what the types involved are.
C++: A Dialog | C++0x Features: Part1 (lambdas, auto, static_assert) , Part 2 (rvalue references) ,
rs_man
rs_man
We have already found out in other forum that it should be like this:

iter=group.begin();
for(int a=0; a{
iter++;
}
(*iter).Verteces.push_back(GetCoord(line));

But now i get this runtime error:
vector iterator not dereferencable.
rip-off
rip-off
You could use this code:
iter = group.begin();std::advance(iter, Group);iter->Verteces.push_back(GetCoord(line));

Or simpler again:
group[Group].Verteces.push_back(GetCoord(line));

As for the error, is the value of "Group" inside the range of the "group" collection? I.e. It should be less than group.size(), and positive.

Finally, having two different variables with the same name, only differing in case is a recipe for confusion. One idea might be to change the name of the vector to a plural, example "groups". Also, "verteces" is incorrect, the correct spelling is "vertices".
rs_man
rs_man
Thank you for reply. My main language is not english, but know I'll know that it's vertices, not verteces. I chaged the name of Group to G and for now it is always 0 so it's defenitely less than group.size() and more then negative numbers. Also I used code:
group[G].Vertices.push_back(GetCoord(line));
This thing gives me an error: vector subscript out of range.
The code with function advance gives me same error as before.
rip-off
rip-off
If "groups" is empty, then even 0 is an invalid subscript. You need to push_back() one or more groups before you can access the elements in the vector.
rs_man
rs_man
Hi, those problems are solved, but I have some new ones. My obj loader draws not right. Before drawing the model i print out all the model vertices, normals, texture coordinates on screen and they are 100% right. But when I use these coordinates with glVertex3f() and draw model on screen all i get is a bunch of randomly placed triangles on the screen, it doesn't look like any model. What could be the problem?
If you need my model drawing function I can paste it here.
Evil Steve
Evil Steve
Sounds like you're passing the vertices to glVertex3f() in the wrong order. If you render the vertices as points instead of triangles, do they seem to be in the correct position?
rs_man
rs_man
How do Irender vertices as points?

Topic Locked

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

Sign in to reply to this topic.