George Green
George Green

Reputation: 4905

C++ pointer to int in loops?

Ok, so I'm quite new to C++ and I'm sure this question is already answered somewhere, and also is quite simple, but I can't seem to find the answer....

I have a custom array class, which I am using just as an exercise to try and get the hang of how things work which is defined as follows:

Header:

class Array {
private:
    // Private variables
    unsigned int mCapacity;
    unsigned int mLength;
    void **mData;

public:
    // Public constructor/destructor
    Array(unsigned int initialCapacity = 10);

    // Public methods
    void addObject(void *obj);
    void removeObject(void *obj);
    void *objectAtIndex(unsigned int index);
    void *operator[](unsigned int index);
    int indexOfObject(void *obj);
    unsigned int getSize();
    };
}

Implementation:

GG::Array::Array(unsigned int initialCapacity) : mCapacity(initialCapacity) {
    // Allocate a buffer that is the required size
    mData = new void*[initialCapacity];
    // Set the length to 0
    mLength = 0;
}

void GG::Array::addObject(void *obj) {
    // Check if there is space for the new object on the end of the array
    if (mLength == mCapacity) {
        // There is not enough space so create a large array
        unsigned int newCapacity = mCapacity + 10;
        void **newArray = new void*[newCapacity];
        mCapacity = newCapacity;
        // Copy over the data from the old array
        for (unsigned int i = 0; i < mLength; i++) {
            newArray[i] = mData[i];
        }
        // Delete the old array
        delete[] mData;
        // Set the new array as mData
        mData = newArray;
    }
    // Now insert the object at the end of the array
    mData[mLength] = obj;
    mLength++;
}

void GG::Array::removeObject(void *obj) {
    // Attempt to find the object in the array
    int index = this->indexOfObject(obj);
    if (index >= 0) {
        // Remove the object
        mData[index] = nullptr;
        // Move any object after it down in the array
        for (unsigned int i = index + 1; i < mLength; i++) {
            mData[i - 1] = mData[i];
        }
        // Decrement the length of the array
        mLength--;
    }
}

void *GG::Array::objectAtIndex(unsigned int index) {
    if (index < mLength) return mData[index];
    return nullptr;
}

void *GG::Array::operator[](unsigned int index) {
    return this->objectAtIndex(index);
}

int GG::Array::indexOfObject(void *obj) {
    // Iterate through the array and try to find the object
    for (int i = 0; i < mLength; i++) {
        if (mData[i] == obj) return i;
    }
    return -1;
}

unsigned int GG::Array::getSize() {
    return mLength;
}

I'm trying to create an array of pointers to integers, a simplified version of this is as follows:

Array array = Array();
for (int i = 0; i < 2; i++) {
    int j = i + 1;
    array.addObject(&j);
}

Now the problem is that the same pointer is used for j in every iteration. So after the loop:

array[0] == array[1] == array[2];

I'm sure that this is expected behaviour, but it isn't quite what I want to happen, I want an array of different pointers to different ints. If anyone could point me in the right direction here it would be greatly appreciated! :) (I'm clearly misunderstanding how to use pointers!)

P.s. Thanks everyone for your responses. I have accepted the one that solved the problem that I was having!

Upvotes: 0

Views: 1848

Answers (5)

Gianluca Ghettini
Gianluca Ghettini

Reputation: 11628

if you simply do

int *array[10];

your array variable can decay to a pointer to the first element of the list, you can reference the i-th integer pointer just by doing:

int *myPtr = *(array + i);

which is in fact just another way to write the more common form:

int *myPtr = array[i];

void* is not the same as int*. void* represent a void pointer which is a pointer to a specific memory area without any additional interpretation or assuption about the data you are referencing to

Upvotes: 1

Chowlett
Chowlett

Reputation: 46667

There are lots of problems with this code.

  1. The declaration void* array = new void*[2] creates an array of 2 pointers-to-pointer-to-void, indexed 0 and 1. You then try to write into elements 0, 1 and 2. This is undefined behaviour
  2. You almost certainly don't want a void pointer to an array of pointer-to-pointer-to-void. If you really want an array of pointer-to-integer, then you want int** array = new int*[2];. Or probably just int *array[2]; unless you really need the array on the heap.
  3. j is the probably in the same place each time through the loop - it will likely be allocated in the same place on the stack - so &j is the same address each time. In any case, j will go out of scope when the loop's finished, and the address(es) will be invalid.

What are you actually trying to do? There may well be a better way.

Upvotes: 1

Vlad from Moscow
Vlad from Moscow

Reputation: 310910

First of all this does not allocate an array of pointers to int

void *array = new void*[2];

It allocates an array of pointers to void.

You may not dereference a pointer to void as type void is incomplete type, It has an empty set of values. So this code is invalid

array[i] = *j;

And moreover instead of *j shall be &j Though in this case pointers have invalid values because would point memory that was destroyed because j is a local variable.

The loop is also wrong. Instead of

for (int i = 0; i < 3; i++) {

there should be

for (int i = 0; i < 2; i++) {

What you want is the following

int **array = new int *[2];

for ( int i = 0; i < 2; i++ ) 
{
   int j = i + 1;
   array[i] = new int( j );
}

And you can output objects it points to

for ( int i = 0; i < 2; i++ ) 
{
   std::cout << *array[i] << std::endl;
}

To delete the pointers you can use the following code snippet

for ( int i = 0; i < 2; i++ ) 
{
   delete array[i];
}

delete []array;

EDIT: As you changed your original post then I also will append in turn my post.

Instead of

Array array = Array();
for (int i = 0; i < 2; i++) {
    int j = i + 1;
    array.addObject(&j);
}

there should be

Array array;
for (int i = 0; i < 2; i++) {
    int j = i + 1;
    array.addObject( new int( j ) );
}

Take into account that either you should define copy/move constructors and assignment operators or define them as deleted.

Upvotes: 2

Daniele
Daniele

Reputation: 831

There are some problems:

1) void *array = new void*[2]; is wrong because you want an array of pointers: void *array[2];

2)for (int i = 0; i < 3; i++) { : is wrong because your array is from 0 to 1;

3)int j = i + 1; array[i] = *j; j is an automatic variable, and the content is destroyed at each iteration. This is why you got always the same address. And also, to take the address of a variable you need to use &

Upvotes: 0

Sean
Sean

Reputation: 62472

I'm guessing you mean:

array[i] = &j;

In which case you're storing a pointer to a temporary. On each loop repitition j is allocated in the stack address on the stack, so &j yeilds the same value. Even if you were getting back different addresses your code would cause problems down the line as you're storing a pointer to a temporary.

Also, why use a void* array. If you actually just want 3 unique integers then just do:

std::vector<int> array(3);

It's much more C++'esque and removes all manner of bugs.

Upvotes: 4

Related Questions