-1

I am relatively new in C++ and didn't expect the behavior shown in code below. Can you, please, advise why is it happening and how to fix it?

DataWrapper.h

#pragma once
class DataWrapper
{
public:
    int myVal;

    DataWrapper();
};

DataWrapper.cpp

#include "DataWrapper.h"

DataWrapper::DataWrapper()
{
    myVal = 1;
}

DataCaller.h

#pragma once
#include "DataWrapper.h"
class DataCaller
{
public:
    DataWrapper* dwRef;

    DataCaller();
    void PrintData();
};

DataCaller.cpp

#include "DataCaller.h"
#include <iostream>

DataCaller::DataCaller()
{
    dwRef = &DataWrapper();
    int extractedVal = dwRef->myVal;
    int dwAddr = (int)dwRef;
    std::cout << "Val In DataCaller Constructor: "<< extractedVal << " dwAddr: " << dwAddr << std::endl;

}

void DataCaller::PrintData()
{
    int extractedVal = dwRef->myVal;
    int dwAddr = (int)dwRef;
    std::cout << "Val In DataCaller PrintData: " << extractedVal << " dwAddr: " << dwAddr << std::endl;

}

Main.cpp

#include <post-processing/DataCaller.h>

int main(int argc, char* argv[])
{
    DataCaller dc = DataCaller();
    dc.PrintData();
    return 0;
}

So I fill the value in DataWrapper constructor , in DataCaller constructor I create DataWrapper pointer, then print DataWrapper pointer's value. I also print this value in regular method of DataCaller class.

When I print it in constructor, it is equal to 1, as written in DataWrapper constructor, but when I print it in DataCaller method - the value is set to rubbish. At the same time "dwAddr" is the same in both cases. Here is an example of my output:

Val In DataCaller Constructor: 1 dwAddr: 2035349268
Val In DataCaller PrintData: -858993460 dwAddr: 2035349268

Can you, please, advise why is it happening and how to fix it?

  • 5
    `dwRef = &DataWrapper();` - What do you expect to happen there? `int dwAddr = (int)dwRef;` is also very strange. What's the goal with this? `auto dwAddr = reinterpret_cast(dwRef);` would be more appropriate. – Ted Lyngmo Dec 23 '22 at 10:02
  • 1
    *and how to fix it?* -- What needs to be fixed? I frankly don't know what your ultimate goal is with that strange code `dwRef = &DataWrapper();`. This is getting into [XY problem territory](https://xyproblem.info/) – PaulMcKenzie Dec 23 '22 at 10:03
  • 5
    `dwRef = &DataWrapper();` creates a temporary object ( `DataWrapper()` ), takes its address and stores it in `dwRef`. At the end of the full expression ( `;` ), the temporary object is destroyed leaving `dwRef` a dangling pointer. Any dereference of `dwRef` is the Undefined behaviour. – Richard Critten Dec 23 '22 at 10:08
  • I expect to reach 2 goals. 1) This object should not be destroyed when the method where it is created is finished. 2) I don't want to initialize DataWrapper obj outside of constructor as both DataWrapper and DataCaller constructors will have params soon. – Alex Dzhus Dec 23 '22 at 10:10
  • Start by changing `DataWrapper* dwRef;` to `std::unique_ptr upDW;` and change `dwRef = &DataWrapper();` to `upDW = std::make_unique();` This will create a new `DataWrapper` object whose lifetime is managed for you. – Richard Critten Dec 23 '22 at 10:15
  • 2
    Is there a reason why `dwRef` is even a pointer? – ChrisMM Dec 23 '22 at 10:17
  • *"in DataCaller constructor I create DataWrapper pointer"* -- you have summed up the root cause of your problem nicely. You focused on creating a pointer, when you should have focused on creating an *object* (whose address would be stored in the pointer). – JaMiT Dec 23 '22 at 11:17
  • @AlexDzhus *"I don't want to initialize DataWrapper obj outside of constructor"* -- this fails to explain why your data member is a pointer (`DataWrapper*`) instead of an object (`DataWrapper`). In fact, I'd view this goal as a reason `dwRef` should **not** be a pointer. – JaMiT Dec 23 '22 at 11:21
  • @JaMiT, when I create it as " DataWrapper dwRef;" in DataCaller.h, it looks like DataWrapper constructor is called before DataCaller constructor, so "dwRef = DataWrapper()" would just override an object that is already constructed, which is not what I want. – Alex Dzhus Dec 23 '22 at 15:33
  • @AlexDzhus *"it looks like DataWrapper constructor is called before DataCaller constructor"* -- it can look that way, but a more accurate description is that the `DataWrapper` constructor is called before the `DataCaller` constructor **body**. Member construction occurs between `DataCaller::DataCaller()` and `{` (a part of the definition that you left empty). -- *"so 'dwRef = DataWrapper()' would just override an object that is already constructed,"* -- Right, don't do that. Use "[this weird colon-member (' : ') syntax in the constructor](https://stackoverflow.com/questions/1711990/)" instead. – JaMiT Dec 23 '22 at 21:28
  • @AlexDzhus See also [XY problem](https://en.wikipedia.org/wiki/XY_problem) – JaMiT Dec 23 '22 at 21:42

1 Answers1

0

As explained by @RichartCritten in the comment, the object created via DataWrapper() is destroyed at the end of the expression

dwRef = &DataWrapper();

which results in undefined behaviour.

If you want to keep the object, one alternative is to make dwRef a non-pointer member variable.

class DataCaller
{
public:
    DataWrapper dwRef;

    DataCaller();
    void PrintData();
};

...

DataCaller::DataCaller()
    : dwRef(/* constructor parameters could be listed here */)
{
    ...
}

Alternatively allocate the object via new; It's preferrable to have a smart pointer manage the calls to new and delete though:

...
#include <memory>
...

class DataCaller
{
public:
    std::unique_ptr<DataWrapper> dwRef;

    DataCaller();
    void PrintData();
};

...

DataCaller::DataCaller()
{
    dwRef = std::make_unique<DataWrapper>(/* constructor parameters could be listed here */);
    ...
}

Note: To print the address via a std::ostream, it's preferrable to cast to void*; this results in a implementation defined output of the address, but usually it's an output format that works well for addresses:

// assuming the second alternative I presented is used
std::cout << "Val In DataCaller PrintData: " << extractedVal << " dwAddr: " << static_cast<void*>(dwRef.get()) << std::endl;
fabian
  • 80,457
  • 12
  • 86
  • 114