C++ pointers (multiple uses inside a class) problems :(

[wyrmmage]

So, I have a class, we'll call it 'anim', and I have a double pointer of type int so that I can have a 2-dimensional dynamic array, like so:

int** theShapes;
theShapes = new int*[3];
theShapes[0] = new int[1];
theShapes[1] = new int[1];
theShapes[2] = new int[1];


I also have a single pointer, which we'll make type double.


double* theInts;
theInts = new int[2];


Now, I want to instantiate an object of type 'anim' and call a method 'copy' that takes a pointer to an already-instantiated anim, like so:

anim theAnim;
anim newAnim;
newAnim.copy(&theAnim);


Now, inside of my copy method, I want to have all of the values for 'theInts' copied, which is simple enough:

theInts = new int[2];
theInts[0] = theAnim->theInts[0];
theInts[1] = theAnim->theInts[1];

but I want the int** theShapes to point to the array theShapes in &theAnim, and not copy that values so that I can access them more efficiently later (I never need to modify them). So, how do I do that? I thought of doing something like this:

theShapes = new int*[1];
theShapes[0] = theAnim->&theShapes;

and then accessing the contents of theShapes like this:

int tempInt = *theShapes[0][0][0];

but it seemed king of clunky, and I'm not even sure if it will work. Should I just store an anim* pointer to the copied anim globally, and then access the information?
I suppose I could make another class called 'copyAnim' or something that would only be for copied animations, since the information in the 'anim' class that I'm copying never gets modified anyway, but I'm not quite sure if that would be a good way to go, or if I could make it work correctly anyway.
Any help would be appreciated :)
-wyrmmage

[akb825]

I see several problems. First of all, your first block of code has a misplaced semi-colon, and your second block assigns an int pointer to a double pointer. Second of all, why are you making new arrays of length 1? Are you planning to have them > 1 at some point? Third of all, copying pointers is generally a bad idea in this case. The reason being when it comes time to free the memory, if you free it from one object it's no longer available to the other object. Also, you're leaking memory by overwriting the pointers in your copy method. (unless you just left out the delete, which once again, would cause problems if it was already copied from something else)

To make this better, I'd look into making less calls to new. One solution would be to keep everything in a flat array, or you could also use some different data structures as well. It's really up to what you need.

As a matter of opinion, I would probably have both a copy constructor (to create a new object with an existing one) and override operator= to copy the contents from one object to another. (both of them taking references, of course, so you don't have recursive copies to make your copies) That way instead of having
anim theAnim;
anim newAnim;
newAnim.copy(&theAnim);
you could have
anim theAnim;
anim newAnim(theAnim);
or
anim theAnim;
anim newAnim;
newAnim = theAnim;

[wyrmmage]

Thanks for the advice :)
My actual code is a bit more complex, so I just typed up some example code that I thought would illustrate the problem (therefore the semicolon in the wrong place :D). In my actual program, yes, the arrays are larger than one :)
The reason that I have the copy as a method instead of using overloading operators or making it a constructor is because I need to call it more than once, and I (might) need the functionality of the plain '=' sign in the future (plus the fact that I just don't like using overloaded operators :sneaky: ). Since the copy method can be called more than once, I (will) be deleting the memory if it is already initialized, so that I'm not re-'new'-ing it. I kind of need a two dimensional array...using a flat array would force me to have have another variable and an array specifically for keeping track of where in the flat array the information is (that doesn't really apply to the example I posted, kind of hard to explain, sorry).
Not quite sure what you meant about the memory leak; could you please explain it in more detail? :)
The anim reference that I'm passing to copy() will never have any of the information deleted from it until the program closes, so I don't have to worry about having invalid pointers, unless I'm passing a reference of an anim that contains pointers, which I'm not.
Thanks for the help :)
Any other suggestions?
-wyrmmage

[PowerMacX]

So, I have a class, we'll call it 'anim', and I have a double pointer of type int so that I can have a 2-dimensional dynamic array,

That's not a 2D array, but an array of arrays. I personally use a declaration like [rows*cols] plus something like [y*rowsize+x] do address a particular element (which is basically what the compiler does for real 2D arrays anyway, and faster than a double indirection)

[wyrmmage]

oh, ya, sorry. Guess it's not technically a real 2D array; the reason why I'm doing it this way is because the arrays that the array is pointing to are different lengths.
-wyrmmage

[akb825]

The memory leak comes when you allocate memory in an anim object, then call copy to copy pointers into that anim object. The original memory would no longer be pointed to and be leaked. If you don't want to leak that memory, you must delete it, but then you run into the problem of if you delete it you may invalidate other pointers. I really think you should do a deep copy each time and manage the memory other than "when it quits it goes away". Unless this is a few objects that really are guaranteed to hang around for all of the program's execution, you really should have a more complete memory management solution.

[JeroMiya]

Also, if you need an array that all instances of the class use, then you may want to use static class variables. If you insist on using C arrays to hold this shared data, you will have to do one of two things: Have static methods to initialize and delete the shared data (you'll have to make sure yourself that no instances of that class exist) or you can use a static reference count, and initialize the data in the constructor if the reference count is 0, and delete it in the destructor if the reference count goes to 0.

(note I haven't compiled this code so may contain typos, syntax errors, etc..)

#ifndef METHODS_H
#define METHODS_H
/**
* \file Methods.h
*/

/**
* Method1 implements the first idea above.
* This method is not thread safe unless the shared data
* is (at the very least) protected by a mutex.
*/
class Method1 {
protected:
/// A value indicating if the shared data is ready
static bool sSharedReady;

/// The shared data
static int* sSharedData;
public:
/**
* Initialize the shared data
* May need to be defined in Methods.cpp I forget.
*/
static void InitializeShared();

/**
* Delete the shared data
*/
static void DeleteShared();

Method1() {
if(!Method1::sSharedReady) {
Method1::InitializeShared();
}
// do other things...
}
};

#endif


(and here is Methods.cpp)

#include "Methods.h"

// sSharedReady is like a global extern, initialized to false at start of program
bool Method1::sSharedReady = false;

// same for sSharedData
int* Method1::sSharedData = NULL;

/**
* Initialize the shared data
* May need to be defined in Methods.cpp I forget.
*/
void Method1::InitializeShared()
{
if(!Method1::sSharedReady) {
Method1::sSharedReady = true;
Method1::sSharedData = new int[SHARED_SIZE];
// not sure if the next line works, may cause conflicts if
// Method1::~Method1() depends on shared data being initialized
atexit(Method1::DestroyShared());
}
}

/**
* Delete the shared data
*/
void Method1::DeleteShared()
{
if(Method1::sSharedReady) {
Method1::sSharedReady = false;
delete []Method1::sSharedData;
}
}



Of course, you can avoid DeleteShared if you make sSharedData a stl vector or at least a custom class you design rather than a naked pointer.

[Hog]

i'd suggest you use STL, i.e.:
std::vector<std::vector<int> > theShapes;

you won't have to allocate or deallocate anything yourself and everything is copied automatically.

[MattDiamond]

i'd suggest you use STL, i.e.:
std::vector<std::vector<int> > theShapes;

you won't have to allocate or deallocate anything yourself and everything is copied automatically.

You also get type-safety, e.g. no assigning array of doubles to array of ints.

Downside is that error messages from misuse can be difficult to work through- that's generally true of templates. Nonetheless, if you stick to the simple containers you will probably come out ahead.

[wyrmmage]

Thanks for all of the suggestions :)
I'm still a bit lost on the memory leaks that you were talking about...does the following code induce a memory leak?

int** theInts;
theInts = new int*[2];
theInts[0] = new int[2];
theInts[1] = new int[1];
theInts[0][0] = 0;
theInts[0][1] = 1;
theInts[1][0] = 2;

int** point;
point = new int*[1];
point[0] = theInts;

delete [] point;
delete [] theInts[0];
delete [] theInts[1];
delete [] theInts;



I'm pretty sure that shouldn't cause a memory leak...am I right? Also: can I access the information in point (before I delete it) like this:

int newInt = *point[0][0][0];
int sInt = *point[0][0][1];
int tInt = *point[0][1][0];


if that doesn't work...how would I access the information stored in my pointer to the double pointer? (the information that point[0] is pointing to).
Thanks :)
-wyrmmage

[ThemsAllTook]

if that doesn't work...how would I access the information stored in my pointer to the double pointer? (the information that point[0] is pointing to).

Like this:

(*point[0])[0][0];

[] binds more tightly than unary *, so *point[0][0][0] means *(point[0][0][0]). You have to explicitly use parentheses to dereference it differently.

[PowerMacX]

I'd hate to be the guy having to maintain your code...

[akb825]

point[0] is leaked in your sample code.

[wyrmmage]

point[0] is leaked in your sample code.
why? What delete command do I need to call to fix it? Do I need to go delete point[0]; delete [] point; ? (thanks for the help, I'm still just a bit lost :( )

PowerMacX:
I'm the only guy on this project; besides, that's why C++ supports /* */ :P

ThemsAllTook:
thanks for the de-referencing code; that would have taken me weeks to figure out on my own :)

-wyrmmage

[akb825]

Look at it this way: every single time you call new, you must also call delete, or you're going to leak memory. Now some of those calls may be hidden, such as in a constructor or destructor of another object, but the compiler itself doesn't make any extra calls itself. (that includes with an array of pointers, or a pointer to a pointer) If you can't see memory leaks in your simple example above or our more complex example, then I suggest that you read up on memory management. Also, as has been mentioned, look up other data structures you can use to avoid so many news within news within news. Those can really complicate things.