-1

I am trying to print an array created inside the class initializer. I have tried a number of ways but my findings are that I have to use pointers for this problem. The code:

#include <iostream>

// arr.h
class Arr
{
private:
    const int sizeVertical;
    const int sizeHorizontal;
    int *ptr;
public:
    Arr(int sizeVertical, int sizeHorizontal);
    void Init();
    void Print();
};

// arr.cpp
Arr::Arr(int _sizeVertical, int _sizeHorizontal)
    :   sizeVertical(_sizeVertical),
        sizeHorizontal(_sizeHorizontal)
{
    int arr2d[sizeVertical][sizeHorizontal];
    ptr = &arr2d[0][0];
}

void Arr::Init()
{
    for(int i=0; i<sizeVertical; i++)
    {
        for(int j=0; j<sizeHorizontal; j++)
        {
            *(ptr + i * sizeHorizontal + j) = i+j;
        }
    }
}

void Arr::Print()
{
    for(int i=0; i<sizeVertical; i++)
    {
        for(int j=0; j<sizeHorizontal; j++)
        {
            std::cout<<*(ptr + i * sizeHorizontal + j)<<std::endl;
        }
    }
}

// main.cpp
int main()
{
    Arr test(4,3);
    test.Init();
    test.Print();
}

Inside the method (Init) where I defined the array I can also print the array in the same way as used in the method (Print). But when trying to print the array in a different method the output looks very strange. The output:

0
22025
955388640
32767
10
0
-1247975061
32668
1
3
-1549041632
22025

Any ideas?

Gilles
  • 3
  • 1
  • 2
    _"I have tried a number of ways but my findings are that I have to use pointers for this problem."_ No, the idiomatic way would be to use a `std::vector>` to solve this. – πάντα ῥεῖ Feb 10 '19 at 11:14

3 Answers3

2

You create local array and assign address of it's first element to the pointer. But the array is gone when constructor finishes, so the address stored in pointer is already invalidated.

Acessing this address in Undefined Behaviour, meaning anything can happen when you run that code.

The best way to handle this is to use std::vector<std::vector<int>>, as suggested in comments:

#include <vector>

class Arr
{
private:
    const int sizeVertical;
    const int sizeHorizontal;
    std::vector<std::vector<int>> arr2d;
public:
    Arr(int sizeVertical, int sizeHorizontal);
    void Init();
    void Print();
};

// arr.cpp
Arr::Arr(int _sizeVertical, int _sizeHorizontal)
    :   sizeVertical(_sizeVertical),
        sizeHorizontal(_sizeHorizontal),
        arr2d(_sizeVertical, std::vector<int>(_sizeHorizontal))
{
}

Init can use [][] for easy access instead of pointer arithmetic.

Note that you no longer need your sizeVertical and sizeHorizontal, you can use arr2d.size() and arr2d[0].size() instead:

for(int i=0; i<arr2d.size(); i++)
{
    for(int j=0; j<arr2d[0].size(); j++)
    {
    }
}

Personally I think named constants are more readable than this, but they would require you to never change the size of vectors. As noted in comments, you can also create such constants locally, in function where you need them.


You could also use dynamic memory allocation, but I don't recommend that. It's difficult to properly manage memory and you will have to follow the Rule of Three/Five

Yksisarvinen
  • 18,008
  • 2
  • 24
  • 52
  • Good one. I think `sizeVertical` and `sizeHorizontal` are no longer required now. You may consider removing them. – Kunal Puri Feb 10 '19 at 11:38
  • @KunalPuri True, but I think they are more readable than `arr2d.size()` and `arr2d[0].size()`. I'll add that in answer for OP to decide. – Yksisarvinen Feb 10 '19 at 11:48
  • You can have local variables for it. For the sake of readability. But I don't think there is a valid reason to have them as data members. They will increase the size of object, just for no reason. – Kunal Puri Feb 10 '19 at 11:51
0

The issue is that your array has been deallocated.

Arr::Arr(int _sizeVertical, int _sizeHorizontal)
    :   sizeVertical(_sizeVertical),
        sizeHorizontal(_sizeHorizontal)
{
    int arr2d[sizeVertical][sizeHorizontal]; // arr2d allocated.
    ptr = &arr2d[0][0];
} // arr2d dies here

Instead, as rightly suggested by πάντα ῥεῖ, You may use std::vector< std::vector<int> >. If you don't want to do it, you may use dynamic memory, though I won't recommend it.

Arr::Arr(int _sizeVertical, int _sizeHorizontal)
    :   sizeVertical(_sizeVertical),
        sizeHorizontal(_sizeHorizontal)
{
    ptr = new int[sizeVertical * sizeHorizontal];
}

Make Sure that you free the dynamic memory allocated to it by having delete in destructor.

Arr::~Arr()
{
    delete[] ptr;
}

This is known as RAII Pattern which guarantees deinitialization.

Edit:

If you don't plan to change the dimensions of array, then I would highly recommend you to use std::array.

#include <iostream>
#include <array>

template<int sizeVertical, int sizeHorizontal>
class Arr
{
    private:
    std::array<std::array<int, sizeHorizontal>, sizeVertical> arr;

    public:
    Arr() { }
    void Init() {
        for (int i = 0; i < sizeVertical; i++) {
            for (int j = 0; j < sizeHorizontal; j++) {
                arr[i][j] = i + j;
            }
        }
    }
    void Print() {
        for (int i = 0; i < sizeVertical; i++) {
            for (int j = 0; j < sizeHorizontal; j++) {
                std::cout << arr[i][j] << " ";
            }

            std::cout << "\n";
        }   
    }
};

int main()
{
    Arr<10, 2> arr;
    arr.Init();

    arr.Print();
}
Kunal Puri
  • 3,419
  • 1
  • 10
  • 22
0

You initialize pointer to int ptr with address of automatic variable arr2d in constructor. Automatic variables allocated in stack, and destroyed automatically once you leave code scope { ... } where they are allocated. So, after you leave your constructor, address in ptr is no longer valid because arr2d is destroyed. You may use dynamic memory allocation in constructor

Arr::Arr(int _sizeVertical, int _sizeHorizontal)
    :   sizeVertical(_sizeVertical),
        sizeHorizontal(_sizeHorizontal)
{
    ptr = new int[sizeVertical * sizeHorizontal];
}

Do not forget to free it in a destructor:

Arr::~Arr(void)
{
    delete[] ptr;
}
Serge Ageyev
  • 437
  • 3
  • 8