0

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!

George Green
  • 4,807
  • 5
  • 31
  • 45
  • replace `array[i] = *j;` with `array[i] = &j;` – mangusta Feb 20 '14 at 11:41
  • and also replace `void *array = new void*[2];` with `int *array = new int*[2];` – mangusta Feb 20 '14 at 11:42
  • finally, replace `i<3` with `i<2` since you have only two elements – mangusta Feb 20 '14 at 11:45
  • Thanks for your response! Ok, the *j was a type in my question, it it actually &j in my code, have just edited! The reason is it void* is because I actually have a wrapped Array class that gets used for lots of things and takes void*, can I not get away with using it that way? And again the 3 thing was another typo! – George Green Feb 20 '14 at 11:46
  • Also how do I use "*" in my comments without making it italic? – George Green Feb 20 '14 at 11:47
  • in your example, you can get away with it, but keep in mind that you will not be able to do pointer arithmetic with your pointers, as they will be `void *` type – mangusta Feb 20 '14 at 11:49
  • yeah, and as mentioned below, declaration should be `void**` not `void*` because you're declaring a pointer to array of pointers – mangusta Feb 20 '14 at 11:54
  • Sorry, I;ve just updated with a more accurate example of what I'm doing. Hopefully with fewer typos :P – George Green Feb 20 '14 at 11:56
  • By the way - practicing is fine, getting to know the basic stuff is important. But please be aware that this code is more C with classes than it is C++. In real C++ (especially true since C++11) you try to minimize pointer arithmetics and other low-level-stuff to an absolute minimum. Your custom array class for example: use `std::vector`, and you're good to go. Getting this stuff right, especially for someone new to C++, is hard. – Excelcius Feb 20 '14 at 12:05
  • @Excelcius I have no intention of actually using this class in any production code. I'm familiar with how to do this using the std::vector, I just like to get a feel for how things work :) – George Green Feb 20 '14 at 12:09
  • @GeorgeGreen Yes, as I said - practicing is good. If you work with C++ you definitely need to know what a pointer is and what `new` does. Just wanted to make sure that you also know that using these low-level features can cause many hard-to-find bugs if you're not careful, especially in large codebases. – Excelcius Feb 20 '14 at 12:14

5 Answers5

4

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.

Sean
  • 60,939
  • 11
  • 97
  • 136
  • I'm using void* because I actually have an Array class that is used for holding pointers to various object types and was trying to use the same class to hold ints. I'm about to update my question with more accurate code. – George Green Feb 20 '14 at 11:52
2

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.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

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

Gianluca Ghettini
  • 11,129
  • 19
  • 93
  • 159
  • I'm glad you agree then, but you should update your answer. `your array variable IS already a pointer` – Simple Feb 20 '14 at 13:13
  • done. check out the edit and tell me if you agree with that. I don't know the meaning of "decay" in this case – Gianluca Ghettini Feb 20 '14 at 13:18
  • the bit I highlighted is what needed fixing. Should just say `your array variable can decay into a pointer to the first element of the list, ...`. Decay is what the conversion from an array to a pointer is called. – Simple Feb 20 '14 at 13:21
1

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.

Chowlett
  • 45,935
  • 20
  • 116
  • 150
0

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 &

Daniele
  • 821
  • 7
  • 18