C++ How to avoid access of members, of a object th

2019-09-18 06:55发布

问题:

What are good practice options for passing around objects in a program, avoiding accessing non initialized member variables.

I wrote a small example which I think explains the problem very well.

#include <vector>
using namespace std;

class container{public:container(){}

    vector<int> LongList;
    bool otherInfo;
};

class Ship
{
public:Ship(){}

    container* pContainer;
};

int main()
{
    //Create contianer on ship1
    Ship ship1;
    ship1.pContainer = new container;
    ship1.pContainer->LongList.push_back(33);
    ship1.pContainer->otherInfo = true;

    Ship ship2;

    //Transfer container from ship1 onto ship2
    ship2.pContainer = ship1.pContainer;
    ship1.pContainer = 0;

    //2000 lines of code further...
    //embedded in 100 if statements....
    bool info = ship1.pContainer->otherInfo;
    //and the program crashes

    return 0;
}

回答1:

The compiler cannot determine if you are introducing undefined behavior like shown in your example. So there's no way to determine if the pointer variable was initialized or not, other than initializing it with a "special value".

What are good practice options for passing around objects in a program, avoiding accessing non initialized member variables.

The best practice is always to initialize the pointer, and check before dereferencing it:

class Ship {
public:
    Ship() : pContainer(nullptr) {}             
        // ^^^^^^^^^^^^^^^^^^^^^ 
    container* pContainer;
};

// ...

if(ship1.pContainer->LongList) {
    ship1.pContainer->LongList.push_back(33);
}

As for your comment:

So there are no compiler flags that could warn me?

There are more simple and obvious cases, where the compiler may leave you with a warning:

int i;
std::cout << i << std::endl;

Spits out

main.cpp: In functin 'int main()':
main.cpp:5:18: warning: 'i' is used uninitialized in this function [-Wuninitialized]
     std::cout << i << std::endl;
                  ^

See Live Demo



回答2:

One good practice to enforce the checks is to use std::optional or boost::optional.

class Ship
{
public:
    Ship() : pContainer(nullptr) {}
    std::optional<container*> Container()
    {
        if(!pContainer)
            return {};
        return pContainer;
    }
private:
    container* pContainer;
};

It will force you (or better: provide a firm reminder) to check the result of your getter:

std::optional<container*> container = ship1.Container();
container->otherInfo; // will not compile
if(container)
    (*container)->otherInfo; // will compile

You would always need to check the result of operation if you use pointers. What I mean is that with optional the situation is more explicit and there's less probability that you as the programmer will forget to check the result.



回答3:

It seems that you are looking for a way to make your code

bool info = ship1.pContainer->otherInfo;

work even though the pContainer may be null.

You can use a sentinel object, which holds some default data:

container default_container;
default_container.otherInfo = false; // or whatever the default is

Then use a pointer to the sentinel object instead of a null pointer:

//Transfer container from ship1 onto ship2
ship2.pContainer = ship1.pContainer;
ship1.pContainer = &default_container; // instead of 0

//2000 lines of code further...
//embedded in 100 if statements....
bool info = ship1.pContainer->otherInfo;

If you use this, you should make sure the sentinel object cannot be destroyed (e.g. make it a static member, or a singleton).

Also, in the constructor, initialize your pointers so they point to the sentinel object:

class Ship
{
public: Ship(): pContainer(&default_container) {}
    ...
};


回答4:

I found an additional solution. It is admittedly not preventing the access of uninitialized objects, but at least the program crashes AND returns an error message, that enables us to correct our mistake. (This solution is particularly for the g++ compiler.)

First of all set the compiler flag _GLIBCXX_DEBUG. Then instead of naked pointer use unique_ptr.

#include <vector>
#include <iostream>
#include <memory>

using namespace std;

class container{

public:container(){}
    int otherInfo = 33;
};

class Ship
{
public:Ship(){}

    std::unique_ptr<container> upContainer;
};

int main()
{
    Ship ship1;

    cout<<ship1.upContainer->otherInfo<<endl;

    return 0;
}

This code will produce an error:

std::unique_ptr<_Tp, _Dp>::pointer = container*]: Assertion 'get() != pointer()' failed.

Hence telling us that we should probably include an if(ship1.upContainer) check.



回答5:

What are good practice options for passing around objects in a program, avoiding accessing non initialized member variables.

Good practice would be to initialize everything in the constructor.

Debatable better practice is to initialize everything in the constructor and provide no way of modifying any members.