CODEWITHSUNDEEP
c++
user2001019
user2001019

Reputation: 33

c++ access private data of the class

I can't figure what I am doing wrong? I have a class, which has private data: 

static const int SIZE = 101;
int *ptr;
int set [SIZE];

And I have 2 constructors. One is a default constructor that set array to 0. And other one that takes 5 arguments and set 5 values in the array to 1. I need to print this array. When I am in constructor everything is working, when I do cout << inside the constructor result is correct. But when I am trying to use function print. Result is garbage. What I am doing wrong?

IntegerSet::IntegerSet() //default constructor
{
    int set[SIZE] = {0};
    ptr = set;
    cout << "Default Constructor: " << endl;
    for (int i =0; i<SIZE ;i++)
    {
        cout << set[i] << " ";
    }
    cout << endl;
}


IntegerSet::IntegerSet(int a, int b, int c, int d, int e)
{
    int set[SIZE] = {0};
    ptr = set;

    ptr[a] = ptr[b] = ptr[c] = ptr[d] = ptr[e] = 1;

    cout << "Constructor with 5 parametrs: " << endl;
    for (int i =0; i<SIZE ;i++)
    {
        cout << ptr[i] << " ";
    }
    cout << endl;
}

void IntegerSet::print() const
{
    bool flag = false;
    cout << "I am in print: " << endl;

    for (int i=0;i<SIZE;i++)
    {
        if (ptr[i]==1)
        {
            cout << i << " ";
            flag = true;
        }
    }
    if (flag == false)
        cout << "-----";
    cout << endl;
}


void main()
{
    IntegerSet s1;
    IntegerSet s2(1,50,10,22,98);

    s2.print();
}

Upvotes: 1

Views: 209

Answers (6)

David G
David G

Reputation: 96810

In addition to my other answer, here is my suggestion related to the quality and maintainablity of your code. You should be using a container such as std::array for compile-time arrays, and also the member-initializer list for the initialization of your data members.

Your were under the misapprehension that the following line of code set the contents of set to 0.

int set[SIZE] = {0};

When rather it overshadows the private data member set in the IntegerSet class and creates a local variable in the constructor. This is one of the reasons why you should be using STL containers for these feats. For example:

#include <array> // for std::array

class IntegerSet
{
private:
    std::array<int, 101> set;
public:
    IntegerSet()
        : set() // zero-initializes each element
    {
        for (auto val : set)
            std::cout << val " ";
        std::cout << std::endl;
    }

    IntegerSet(int a, int b, int c, int d, int e)
        : set()
    {
        set[a] = set[b] = set[c] = set[d] = set[e] = 1;

        for (auto val : set)
            std::cout << val " ";
        std::cout << std::endl;
    }
};

Upvotes: 1

Subbu
Subbu

Reputation: 2101

array set is local to the constructor . Hence the pointer ptr which points to the set ,later points to NULL as the array set does not exist when the control goes out of the constructor .

Use the array set which is instance variable so that you have the same instance of set in both the constructors as well as in the print method .

IntegerSet::IntegerSet()                                            //default constructor
{
    set[SIZE-1] = {0};
    ptr = set;
    cout << "Default Constructor: " << endl;
    for (int i =0; i<SIZE ;i++)
    {
        cout << set[i] << " ";
    }
    cout << endl;
}


IntegerSet::IntegerSet(int a, int b, int c, int d, int e)
{
    set[SIZE-1] = {0};
    ptr = set;

    ptr[a] = ptr[b] = ptr[c] = ptr[d] = ptr[e] = 1;

    cout << "Constructor with 5 parametrs: " << endl;
    for (int i =0; i<SIZE ;i++)
    {
        cout << ptr[i] << " ";
    }
    cout << endl;
}

Upvotes: 0

David G
David G

Reputation: 96810

Here is the explanation for your garbage output:

ptr[a] = ptr[b] = ptr[c] = ptr[d] = ptr[e] = 1;
// ...
for (int i = 0; i < SIZE; i++)
{
    cout << ptr[i] << " ";
}

In your main method, you used the arguments 1, 50, 10, 22, and 98. So the first line in the above example is equivalent to:

ptr[1] = ptr[50] = ptr[10] = ptr[22] = ptr[98] = 1;

and there's nothing wrong with that. The problem is what happens below it. In your for() loop you're attempting to iterate over ptr from its first index (0) to SIZE. You've only given the values for 5 places in the array, so the others are uninitialized. This is causing Undefined Behavior for your program and is the reason for the garabage output.

You should print the values using the indexes separately.

But until you can tell us what the reason is for you doing this, I can't suggest a better alternative method.

Upvotes: 0

Aakash
Aakash

Reputation: 1900

You are setting ptr to set that is defined in your constructor and not the set that is the class variable. change the following in your constructor : 

int set[SIZE] = {0};
ptr = set;

to 

set[SIZE-1] = {0};
ptr = set;

Upvotes: 1

thelamb
thelamb

Reputation: 486

You put int set[SIZE] = {0}; in the constructor as well, which defines a local set on the stack (this 'shadows' the private member variable which is also called set).

In case you're trying to do set[SIZE] = 0 (set the last element of set to 0) then you have a second mistake: You are accessing set out of bounds (arrays in C and C++ are 0-indexed. So an array of size 5 has valid index (0, 1, 2, 3 and 4)). You should do set[SIZE-1] = 0. Or better yet, use std::vector or std::array (C++11) instead of C-style arrays.

Upvotes: 0

David Schwartz
David Schwartz

Reputation: 182761

Each of your constructors declares a new array called set which shadows the class member.

Upvotes: 6

Related Questions