CODEWITHSUNDEEP
c++listvectorstlshuffle
wmac
wmac

Reputation: 1063

random_shuffle list by copying it to a vector and back

I am trying to shuffle a list by copying it to a vector first and then back to the empty list.

vector<Agent*> tmpVector(agents_.size());
copy(agents_.begin(), agents_.end(), tmpVector.begin());
random_shuffle(tmpVector.begin(), tmpVector.end());
agents_.clear();
copy(tmpVector.begin(), tmpVector.end(),agents_.begin());

The program crashes with a run-time error: list iterator not dereferencable

1- What is wrong with the code.

2- The list contains pointers. I assume nothing would go wrong if the above approach works (because pointer values won't change and the allocated variables can still be referenced with them later) , right?

thanks.

Upvotes: 1

Views: 592

Answers (3)

Yuushi
Yuushi

Reputation: 26040

The problem comes from the last two lines:

agents_.clear();
copy(tmpVector.begin(), tmpVector.end(),agents_.begin());

Specifically, you call clear() on the list, which destroys all elements and leaves the list with a size of 0, and then try to access begin(). Trying to dereference begin() then turns into undefined behaviour.

You can clean this code up quite a bit:

vector<Agent*> tmpVector(agents_.begin(), agents_.end());
random_shuffle(tmpVector.begin(), tmpVector.end());
copy(tmpVector.begin(), tmpVector.end(),agents_.begin());

Will do what you want. The first copy in unnecessary, vector has a constructor taking 2 iterators you can use in this case.

Upvotes: 1

Loki Astari
Loki Astari

Reputation: 264411

When you clear() the tmp array you set its size to zero.

The copy expects the destination to have enough space already set up (ie the size must be correct).

The first alternative is to use back inserter:

agents_.clear();
std::copy(tmpVector.begin(), tmpVector.end(), std::back_inserter(agents_));

But since you are using pointers in the destination.
It will copy over them with no problem. So it is probably easier to just remove the clear.

// agents_.clear();
// The copy will copy over the old values with no problems.
copy(tmpVector.begin(), tmpVector.end(),agents_.begin());

Upvotes: 1

templatetypedef
templatetypedef

Reputation: 372814

I think the issue is in these two lines:

agents_.clear();
copy(tmpVector.begin(), tmpVector.end(),agents_.begin());

This first line clears out the agents_ list, so it's now empty. The next line then tries to replace the sequence of elements stored in agents_ starting at the first element with the contents of the range [tmpVector.begin(), tmpVector.end()). This causes undefined behavior, because there are no elements in the list.

To fix this, try removing the call to agents_.clear() from the previous line. This will cause the copy call to overwrite the existing elements in the agents_ list with the appropriately shuffled values.

Hope this helps!

Upvotes: 2

Related Questions