CODEWITHSUNDEEP
c++pointerspass-by-reference
ABCplus
ABCplus

Reputation: 4021

Passing data by reference to build it

Under C++ I usually ran into an error. Suppose I have the following classes:

class ClassData
{
public:
    ClassData() { a=-1; }

public:
    int a;
};

class MyClass
{
public:
    MyClass() { m_classData = 0; }
    MyClass(ClassData* classData) { m_classData = new ClassData(*classData); m_classData->a=1; }

    inline bool getClassData(ClassData* classData) {
        if (m_classData) { classData = m_classData; return true; }
        else return false;
    }

private:
    ClassData* m_classData;
};

Now in the main I run this test code:

ClassData cdata;
MyClass* m_myClass = new MyClass(&cdata);

int value = 0;
ClassData classData;
if (m_myClass->getClassData(&classData))
    value = classData.a;

If the code is run, value is equal to -1 (instead of 1).

First question: why I've not been able to take a reference to ClassData in MyClass? While I'm thinking to have the ClassData object of m_myClass (like doing m_myClass->m_classData if it would be public), in reality I'm calling the member a on local ClassData object.

I was thinking that the cause was that I cannot change the local variable address, but if I change the code into this:

ClassData* classData=0;
if (m_myClass->getClassData(classData))
    value = classData->a;

I get my program crashing.

Second question: what is the correct way to manage this situation? Is the issue located in the program main function or in how getClassData is defined giving room for mistakes?

Upvotes: 0

Views: 60

Answers (2)

Thomas
Thomas

Reputation: 5138

Using values instead of pointers simplifies things and will also remove your memory leak. See live example

#include <iostream>

class ClassData
{
public:
    ClassData() { a=-1; }

public:
    int a;
};

class MyClass
{
public:
    MyClass() {  }
    explicit
    MyClass(ClassData const& classData) { 
        m_classData = classData; 
        m_classData.a=1; 
    }

    ClassData const& getClassData() const {
        return m_classData;
    }

private:
    ClassData m_classData;
};

int main()
{
    ClassData classData;
    MyClass myClass( classData );

    classData = myClass.getClassData();
    int value = classData.a; 

    std::cout << value;
}

Upvotes: 0

Beta
Beta

Reputation: 99094

Here:

bool getClassData(ClassData* classData) {
  if (m_classData) { classData = m_classData; return true; }
  else return false;
}

Notice that classData is a local variable. It dies when the function terminates, so the function really does nothing except report whether its m_classData is null or not.

So in the first case:

ClassData classData; // <-- classData.a is -1
if (m_myClass->getClassData(&classData))
  value = classData.a; // <-- classData.a is still -1

In the second case:

ClassData* classData=0;      // classData is null
if (m_myClass->getClassData(classData))
    value = classData->a;    // classData is still null

you dereference a null pointer, which is Undefined Behavior. You're lucky all it does is crash.

A correct (but still unsafe) way to do it is like this:

bool getClassData(ClassData* classData) {
  if (m_classData!=0 && classData!=0)
    { classData->a = m_classData->a; return true; }
  else return false;
}

I say "still unsafe" because this function cannot really verify that these pointers point to valid data structures. To avoid this danger you must refrain from using pointers so much, and look into references.

Upvotes: 1

Related Questions