Reputation: 51
I'm learning C++ and I'm wondering if anyone can explain some strange behaviour I'm seeing.
I'm currently learning memory management and have been playing around with the following code:
#include <iostream>
#include <vector>
#include <cmath>
using namespace std;
// pass back by pointer (old C++)
const int array_size = 1e6; // determines size of the random number array
vector<int> *RandomNumbers1()
{
vector<int> *random_numbers = new vector<int>[array_size]; // allocate memory on the heap...
for (int i = 0; i < array_size; i++)
{
int b = rand();
(*random_numbers).push_back(b); // ...and fill it with random numbers
}
return random_numbers; // return pointer to heap memory
}
int main (){
vector<int> *random_numbers = RandomNumbers1();
for (int i = 0; i < (*random_numbers).size(); i++){
cout << (*random_numbers)[i] + "\n";
}
delete random_numbers;
}
What I'm trying to do is get a pointer to a vector containing random integers by calling the RandomNumbers1()
function, and then print each random number on a new line.
However, when I run the above code, instead of printing out a random number, I get all sorts of random information. It seems as though the code is accessing random places in memory and printing out the contents.
Now I know that I'm doing something stupid here - I have an int
and I am adding the string "\n"
to it. If I change the code in main()
to the following, it works fine:
int main (){
vector<int> *random_numbers = RandomNumbers1();
for (int i = 0; i < (*random_numbers).size(); i++){
cout << to_string((*random_numbers)[i]) + "\n";
}
}
However I just can't understand the behaviour I'm getting with the "wrong" code - i.e. how adding the string "\n"
to (*random_numbers)[i]
causes the program to access random areas of memory, instead of where my pointer is pointing to. Surely I have de-referenced the pointer and accessed the element at position i
before "adding" "\n"
to it? So how is the program instead accessing a totally different memory address?
Upvotes: 0
Views: 149
Reputation: 595712
There are several issues with your code.
you are using delete
instead of delete[]
to free the array allocated with new[]
.
you are creating an array of 1000000 vector
s, but populating only the 1st vector
with 1000000 integers. You probably meant to create just 1 vector
instead.
you can and should use the ->
operator when accessing an object's members via a pointer. Using the *
and .
operators will also work, but is more verbose and harder to read/code for.
you are trying to print a "\n"
after each number, but you are using the +
operator when you should be using the <<
operator instead. You can't append a string literal to an integer (well, you can, but it will invoke pointer arithmetic and thus the result will not be what you want, as you have seen).
With that said, try something more like this:
#include <iostream>
#include <vector>
#include <cmath>
using namespace std;
const int array_size = 1e6; // determines size of the random number array
vector<int>* RandomNumbers1()
{
vector<int> *random_numbers = new vector<int>;
random_numbers->reserve(array_size);
for (int i = 0; i < array_size; ++i)
{
int b = rand();
random_numbers->push_back(b);
}
return random_numbers;
}
int main (){
vector<int> *random_numbers = RandomNumbers1();
for (size_t i = 0; i < random_numbers->size(); ++i){
cout << (*random_numbers)[i] << "\n";
}
/* alternatively:
for (int number : *random_numbers){
cout << number << "\n";
}
*/
delete[] random_numbers;
}
However, if you are going to return a pointer to dynamic memory, you really should wrap it inside a smart pointer like std::unique_ptr
or std::shared_ptr
, and let it deal with the delete
for you:
#include <iostream>
#include <vector>
#include <cmath>
#include <memory>
using namespace std;
const int array_size = 1e6; // determines size of the random number array
unique_ptr<vector<int>> RandomNumbers1()
{
auto random_numbers = make_unique<vector<int>>();
// or: unique_ptr<vector<int>> random_numbers(new vector<int>);
random_numbers->reserve(array_size);
for (int i = 0; i < array_size; ++i)
{
int b = rand();
random_numbers->push_back(b);
}
return random_numbers;
}
int main (){
auto random_numbers = RandomNumbers1();
for (size_t i = 0; i < random_numbers->size(); ++i){
cout << (*random_numbers)[i] << "\n";
}
/* alternatively:
for (int number : *random_numbers){
cout << number << "\n";
}
*/
}
Though, in this case, there is really no good reason to create the vector
dynamically at all. 99% of the time, it is unnecessary and unwanted to use standard containers like that. Since the vector
manages dynamic memory internally, there is no reason for the vector
itself to also be created in dynamic memory. Return the vector
by value instead, and let the compiler optimize the return for you.
#include <iostream>
#include <vector>
#include <cmath>
using namespace std;
const int array_size = 1e6; // determines size of the random number array
vector<int> RandomNumbers1()
{
vector<int> random_numbers;
random_numbers.reserve(array_size);
for (int i = 0; i < array_size; ++i)
{
int b = rand();
random_numbers.push_back(b);
}
return random_numbers;
}
int main (){
vector<int> random_numbers = RandomNumbers1();
for (size_t i = 0; i < random_numbers.size(); ++i){
cout << random_numbers[i] << "\n";
}
/* alternatively:
for (int number : random_numbers){
cout << number << "\n";
}
*/
}
Upvotes: 2
Reputation: 75062
"\n"
is a string literal. It is an array and it is converted to a pointer pointing at its first element in your expression.
(*random_numbers)[i]
is an integer.
Adding a pointer to an integer means that advance the pointer by the integer.
This will drive the pointer to out-of-range because "\n"
has only 2 elements ('\n'
and '\0'
) but the numbers returnd from the rand()
function are likely to be larger than 2.
Upvotes: 4