Sign in to follow this  

Struct Pointer is an Undefined Object?

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

If you intended to correct an error in the post then please contact us.

Recommended Posts

I don't understand what I'm doing wrong. I'm trying to write a program to read (at the moment) simply vertices from a model file. I represent each model being read as a model object, and then have inside that model object, many structs containing arrays of data that the model holds. I procedurally read the model line by line until I reach a new geoset, and then initialize a new geoset object in the model object's geosets array. I then add vertices to that new geoset. However, although my program allows me to do this, it crashes on the line that replaces the old geoset in the geoset array with the new geoset (with new vertices in it). The thought process for doing this was to read vertices into a temporary geoset object, and then add that object into the already existing geosets array - however, this is not working at all for me, I get a "Object reference not set to an instance of an object." error thrown on the line that says OldGeosetArray[ArrayIndex] = NewTempGeoset; Can someone help me here? I've posted the code I used below (with some parts removed for simplicity of reading).
        string line; // a temporary string object that is filled with a Getline function to read a line from the file
	double tempDouble; //  a temporary double variable that numbers can be read into using the IFStream's extraction operator
	unsigned int tempUInt; // a temporary integer variable

		TheMDLData.TheNumMDLGeosets ++;
		MDLGeoset* OldGeosets = new MDLGeoset[TheMDLData.TheNumMDLGeosets ++];
		OldGeosets = TheMDLData.TheMDLGeosets;
		MDLGeoset* NewGeoset = new MDLGeoset;
		(*NewGeoset).MDLGeoset_NumMDLVertices = tempUInt; // The following function reads the number of vertices that this particular geoset has
                ReadFromMDL(GeoVertNum);

		(*NewGeoset).MDLGeoset_Vertices = new MDLVertex[(*NewGeoset).MDLGeoset_NumMDLVertices]; // creates the array of vertices

		// Here we start reading in the actual vertices of the geoset into the MDLGeoset.MDLGeoset_MDLVertex array
		for (unsigned int x = 0; x < (*NewGeoset).MDLGeoset_NumMDLVertices; x++)
		{
                        TheMDLFile >> (*NewGeoset).MDLGeoset_Vertices[x].MDLVertex_XVert;	
			TheMDLFile >> (*NewGeoset).MDLGeoset_Vertices[x].MDLVertex_YVert;
			TheMDLFile >> (*NewGeoset).MDLGeoset_Vertices[x].MDLVertex_ZVert;
		}
		// Now we're done reading vertices

		// Let's save the geoset into memory for the computer
		OldGeosets[TheMDLData.TheNumMDLGeosets] = (*NewGeoset); // NOTE - PROGRAM CRASHES ON THIS LINE!!!

		// Now that we're done with that, let's update all of the geosets into the master model
		TheMDLData.TheMDLGeosets = OldGeosets;
		delete NewGeoset;
		delete OldGeosets;
	}


Share this post


Link to post
Share on other sites

MDLGeoset* OldGeosets = new MDLGeoset[TheMDLData.TheNumMDLGeosets ++];
OldGeosets = TheMDLData.TheMDLGeosets;
MDLGeoset* NewGeoset = new MDLGeoset;
(*NewGeoset).MDLGeoset_NumMDLVertices = tempUInt;

Your first line allocates an array. Your second line throws that array away, replacing it with something else. The third line then creates a single MDLGeoset object. The fourth line initializes a part of that object with the uninitialized tempUInt object. I'm betting line two is your problem [if not, it should be...it causes a memory leak], but line four is also quite buggy.

CM

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:

MDLGeoset* OldGeosets = new MDLGeoset[TheMDLData.TheNumMDLGeosets ++];
OldGeosets = TheMDLData.TheMDLGeosets;
MDLGeoset* NewGeoset = new MDLGeoset;
(*NewGeoset).MDLGeoset_NumMDLVertices = tempUInt;


Your first line allocates an array. Your second line throws that array away, replacing it with something else. The third line then creates a single MDLGeoset object. The fourth line initializes a part of that object with the uninitialized tempUInt object. I'm betting line two is your problem [if not, it should be...it causes a memory leak], but line four is also quite buggy.

CM

I'll try modifying those lines you specified, however, I fail to see how the second line is the problematic one.
The first line allocates a new array of geosets of size one geoset more than previously existed (TheNumMDLGeosets++). The theory behind this implementation was to create an empty array of geosets, then fill that array up with the preexisting geosets such that I can merely fill the final geoset with the one being read from the file.
The idea behind the second line was to change the memory location that OldGeosets was pointing to from a newly initialized object (yeah, I see the memory leak) to point to the already existing TheMDLData.TheMDLGeosets - essentially copying TheMDLGeosets into the OldGeosets object.
Also, line four is not the problem, I merely failed to include the portion of the program that reads from the file header the number of vertices and stores that number into tempUInt.

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
I'll try modifying those lines you specified, however, I fail to see how the second line is the problematic one.
The first line allocates a new array of geosets of size one geoset more than previously existed (TheNumMDLGeosets++). The theory behind this implementation was to create an empty array of geosets, then fill that array up with the preexisting geosets such that I can merely fill the final geoset with the one being read from the file.
The idea behind the second line was to change the memory location that OldGeosets was pointing to from a newly initialized object (yeah, I see the memory leak) to point to the already existing TheMDLData.TheMDLGeosets - essentially copying TheMDLGeosets into the OldGeosets object.


Pointer A = Pointer B is NOT the same as copy the stuff pointed to by B into the location pointed to by A. For that, and assuming you have a proper copy constructor, you would need:

*OldGeosets = *TheMdlData.TheMdlGeosets;

However, I don't think that's really what you want to do anyway. You are basically describing a dynamically sized array. In this case, std::vector would be a much better choice.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
After receiving a TON of errors in program crashes, mostly consisting of:
Quote:

Unhandled Exception: System.NullReferenceException: Object reference not set to
an instance of an object.


I have concluded that the problem lies not in the writing of data into the OldGeosets object, but at the reading of data from the TheMDLData.TheMDLGeosets object.

Further research must be performed to fully understand what is going on here...

-DG

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:

However, I don't think that's really what you want to do anyway. You are basically describing a dynamically sized array. In this case, std::vector would be a much better choice.

I'm trying to steer clear of using more classes and objects than I need to, however, wouldn't that solution be the same as saying:

for (unsigned int x = 0; x < TheMDLData.TheNumMDLGeosets; x++) // To copy the preexisting geosets into the newly created array of geosets one by one
{
OldGeosets[x] = TheMDLData.TheMDLGeosets[x];
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
Quote:

However, I don't think that's really what you want to do anyway. You are basically describing a dynamically sized array. In this case, std::vector would be a much better choice.

I'm trying to steer clear of using more classes and objects than I need to, however, wouldn't that solution be the same as saying:
*** Source Snippet Removed ***


No. When you add to the end of a vector, the existing data is copied to a new location only if the existing data block isn't large enough. std::vector uses size doubling when reallocating space, so you will eventually reach a point where the memory copying isn't needed.

As to using more classes and objects than necessary, I would think using std::vector would actually reduce the number, not increase it. It will also greatly simplify your code, since all of the grunt work of allocating/reallocating/copying is done for you.

If you don't need random access to the MDLGeoset collection, then you can use std::list and avoid the potential reallocate/copy altogether.

Other than as a learning exercise, there is really little point in manually implementing something that already exists (and is thouroughly tested) in the standard c++ library.

Share this post


Link to post
Share on other sites

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

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this