Accessing private pointer variables in C++ class methods

Question

I have this code:

#include<iostream>

class Test {
  private:
    int iNum;
    int *ptr;

  public:
    Test(int iFirst, int iSecond);
    void setNum(int iValue);
    int getNum();
    int getFirstNum();
    int getSecondNum();
};

Test::Test(int iFirst, int iSecond) {
  int *ptr = (int *)malloc(sizeof(int) * 2);
  ptr[0] = iFirst;
  ptr[1] = iSecond;
}

void Test::setNum(int iValue) {
  iNum = iValue;
}

int Test::getNum() {
  return iNum;
}

int Test::getFirstNum() {
  return ptr[0];
}

int Test::getSecondNum() {
  return ptr[1];
}

int main() {
  Test oTest(3,4);

  std::cout << oTest.getFirstNum() << std::endl;
  return 0;
}

I just don't understand why I am getting junk value when I try to return ptr[0] using getFirstNum() method. Please enlighten me on how this behaves in memory and possible ways to fix it. Thank youi

in constructor you initialize a local variable `int *ptr` instead member `ptr`. Just write `ptr = (int *)malloc(sizeof(int) * 2);` — nikniknik2016, Jan 09 '17 at 06:15
In constructor, `int *ptr = (int *)malloc(sizeof(int) * 2);` the local `ptr` shadows the class member `ptr` — Danh, Jan 09 '17 at 06:16
Thus, access `ptr` in `getFirstNum` yield an undefined behavior — Danh, Jan 09 '17 at 06:16
Oh no.. I am so embarrassed. haha.. crazy mistake.. Thank you very much. Didn't notice that. — Redis1001, Jan 09 '17 at 06:17
Don't forget to free that mempory and also keep an eye out for the Rule of Three. [What is the Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) Funny you should ask that. Just linked the answer. — user4581301, Jan 09 '17 at 06:21
Surely.. need to delete. Thank you. I just didn't include it here. Thanks! Thanks! — Redis1001, Jan 09 '17 at 06:24
Better to not `new` the array in the first place. Rule of zero. — juanchopanza, Jan 09 '17 at 07:03

score 2 · Answer 1 · answered Jan 09 '17 at 06:16

2

Your mistake is in your constructor:

int *ptr = (int *)malloc(sizeof(int) * 2);

Here, you're declaring a variable named ptr. However, you already have a variable named ptr as a member of your class. This is called shadowing. Your compiler will always use the version of the variable with the narrowest scope, which in this case is the one that you declare in the constructor. That gets thrown away when your constructor returns, and the ptr in the class never gets touched.

To fix this, don't re-declare the variable:

ptr = (int *)malloc(sizeof(int) * 2);

answered Jan 09 '17 at 06:16

IanPudney

5,941
1
24
39

I don't understand why don't you close this question as a typo instead – Danh Jan 09 '17 at 06:18
@Danh It's not a typo, it's an understando – user253751 Jan 09 '17 at 06:31
@immibis anyway, this question still should be closed, _While similar questions may be on-topic here, this one was resolved in a manner unlikely to help future readers. This can often be avoided by identifying and closely inspecting the shortest program necessary to reproduce the problem before posting._ – Danh Jan 09 '17 at 06:49

score 2 · Answer 2 · answered Jan 09 '17 at 06:16

2

int *ptr = (int *)malloc(sizeof(int) * 2);

Inside constructor you have created a new pointer ptr thus shadowing the class member ptr. As it's a new pointer, your class member ptr is never initialized. Remove int * from this line and that will initialize class member ptr.

ptr = (int *)malloc(sizeof(int) * 2);

Another thing is since you are allocating memory dynamically for ptr you must delete it in destructor. Otherwise you will leak memory.

answered Jan 09 '17 at 06:16

taskinoor

45,586
12
116
142

I don't understand why don't you close this question as a typo instead – Danh Jan 09 '17 at 06:17
@Danh I'm not sure whether this is a typo. To me it seems that OP doesn't have the idea of shadowing member variables which needs to be clarified. – taskinoor Jan 09 '17 at 06:19
If you take 1 minute to read OP's comment, he indicated it clearly that it's just a typo – Danh Jan 09 '17 at 06:20
Ya, saw his comment just now :-(. – taskinoor Jan 09 '17 at 06:21

score -1 · Answer 3 · answered Jan 09 '17 at 06:21

#include<iostream>

class Test {
private:
    int iNum;
    int *ptr;

public:
    Test(int iFirst, int iSecond);
    ~Test();
    void setNum(int iValue);
    int getNum();
    int getFirstNum();
    int getSecondNum();
};

Test::Test(int iFirst, int iSecond) {
    ptr = new int[2];
    ptr[0] = iFirst;
    ptr[1] = iSecond;
}

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

void Test::setNum(int iValue) {
    iNum = iValue;
}

int Test::getNum() {
    return iNum;
}

int Test::getFirstNum() {
    return ptr[0];
}

int Test::getSecondNum() {
    return ptr[1];
}

int main() {
    Test oTest(3, 4);

    std::cout << oTest.getFirstNum() << std::endl;
    return 0;
}

You were treating it like c, and not c++. I fixed the issues I saw, you were using malloc and not new, plus you were making a new int*ptr and not using the member one, hence when you called it in a getter function it was not valid.

Also a destructor and delete the memory.

I'm not sure why I got downvoted, by saying he needs to delete something he dynamically allocated. The default dtor is not going to do that. He could use a smart pointer, etc. instead if he wanted to. — CodeSlapper, Jan 09 '17 at 06:37
In introducing a "fix" your also introduced serious bugs. Satisfying the rule of three would be the bare minimum required, but you could also do away with the explicit dynamic allocation entirely (rule of zero.) — juanchopanza, Jan 09 '17 at 06:41

Accessing private pointer variables in C++ class methods

3 Answers3