Please
Please

Reputation: 296

Values being overwritten in array of pointers

I'm Java guy trying to solve discrete knapsack problem in c++. However, I'm having trouble with pointers. I have an object with a field

Item ** items;

representing array of items to choose from. I also created a method to add an item which works like insertion sort (at least I hope so).

void Knapsack::addItem(Item item) {

int k = itemCount - 1;

if (this->items[k] != NULL) {
    return;
}

while (k > 0 && this->items[k - 1] == NULL) {
    k--;
}

if (k == 0) {
    this->items[0] = &item;
} else {
    int i = 0;

    while (i < k && item < *(this->items[i])) {
        i++;
    }

    for (int n = k; n > i; n--) {
        this->items[n] = this->items[n - 1];
    }

    this->items[i] = &item;
}
}

Later, in my main I invoke the method by

knapsack->addItem(*(new Item(values.at(0), values.at(1))));

values being a vector of ints. The method itself seems to work fine, however, debugger shows that everytime I invoke the method with new Item, the previous values already put in my array are set to the same values as the new item. (ex. if items[0] has value of 5, and I invoke the method with an item valued as 10, the items[0] instantly is set to 10).

Why are the values overwritten? I am creating a new object everytime I invoke the method.

EDIT:

Problem was fixed by replacing

this->items[0] = &item;
this->items[i] = &item;

with

this->items[0] = new Item(item.getWeight(), item.getValue());
this->items[i] = new Item(item.getWeight(), item.getValue());

SECOND EDIT:

The answer shows better (and probably correct) way to do this. Now the function takes a pointer instead of an object.

void Knapsack::addItem(Item * item);
this->item[i] = item;
knapsack->addItem(new Item(values.at(0), values.at(1)));

Upvotes: 0

Views: 1153

Answers (1)

Hatted Rooster
Hatted Rooster

Reputation: 36463

You are storing a pointer to a temporary copy of an Item object in the array in your addItem function, once the function returns the temporary object will be destroyed and you will be left with an invalid pointer. Make sure to allocate an Item object on the heap and passing a pointer to your addItem function or just use a vector of type std::vector<Item> and save your objects in there.

Upvotes: 2

Related Questions