0

I just wrote code to copy a vector value of another vector and when I edit the second vector the original one should be changed.
Here is the code confusing me:

#include <iostream>
#include <string>
#include <vector>
using namespace std;

int main() {
    vector<int>a = {1,2,3,4,5};
    vector<int>* b = new vector<int>(6);
    b = &a;
    (*b).push_back(7);
    for (int i = 0; i <= 6; i++) {
        cout << a[i];
    }
    cout << endl;
    for (int i = 0; i <= 6; i++) {
        cout << (*b).at(i);
    }
    delete b;
}

the question is why I get this result when I run the program:

1234570
123457

Why don't I get the last zero in both of them?

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
Furat Mostafa Smadi
  • 29
  • 5
  • 1
    `b=&a;` -- You've just created a memory leak with that line. – PaulMcKenzie Apr 10 '20 at 19:46
  • 1
    Why are you using `new` to create a vector? – Cory Kramer Apr 10 '20 at 19:47
  • 1
    `cout< – PaulMcKenzie Apr 10 '20 at 19:48
  • @PaulMcKenzie It's works but why? – Furat Mostafa Smadi Apr 10 '20 at 19:52
  • Accessing a vector's elements out-of-bounds using `[ ]` is undefined behavior -- anything can happen, include "works". Using `at()` the vector will check the value to see if it is within bounds, and if not, will throw an `std::out_of_bounds` exception. – PaulMcKenzie Apr 10 '20 at 19:53
  • 3
    *"I just write a code to copy a vector value of another vector"* - so, do that. `std::vector b = a;` . Done. There is no reason whatsoever in this code for dynamic allocation of `b`. And `std::vector` is already dynamic internally, so putting the vector itself on the heap offers little (i.e. any) reward. – WhozCraig Apr 10 '20 at 20:28

2 Answers2

2

First of all, you have a serious memory leak issue. You are creating a new vector on the heap memory, and then just throw it away, without cleaning it:

vector<int> *b = new vector<int>(6);
b = &a;

Whenever you use the new keyword, remember to use delete as well:

vector<int> *b = new vector<int>(6);
delete b;
b = &a;

Or simply- avoid using new from the first place, if you don't need to (And if you really need, use smart pointers):

vector<int> *b = &a;

For your question, a & b are both contain 6 elements, and when you try to print their 7th element, you trigger Undefined Behavior (UB), which means you can see any result after 123457, that you'll never be able to predict. You are actually trying to access an area of memory that might not belong to you.

To avoid this UB, you can use .size() property of your vector:

for (int i = 0; i < a.size(); i++)

Or in b:

for (int i = 0; i < b->size(); i++)

As @Deduplicator suggested, simplify your code by using foreach/range loops where you can:

for (auto elem : b) {
    std::cout << elem;
}

Or for more complex collections:

for (auto &elem : collection) { /*...*/ }

Or using C++ standard library algorithm for_each():

// Don't forget to #include <algorithm>

std::for_each(b.begin(), b.end(), (int a)[] { std::cout << a; });
Deduplicator
  • 44,692
  • 7
  • 66
  • 118
Coral Kashri
  • 3,436
  • 2
  • 10
  • 22
0

There are several issues with your code. Most importantly, you have UB on this line

for(int i=0;i<=6;i++){
  cout<<a[i];
}

a only contains 6 elements, so you are indexing out of bounds. Don't do that, it's UB. If this happens, then your entire program is broken. It can do anything (including printing out the correct result).

Secondly, on this line

vector<int>*b=new vector<int>(6);

you are allocating a vector on the heap. Then on the next line

b=&a;

you are re-pointing b, thereby leaking the previously allocated vector.

Also, you are then deleteing b, but it's not pointing to memory that was newed.

Last, but not least, please don't do using namespace std.

cigien
  • 57,834
  • 11
  • 73
  • 112