Is it OK to pass an unreferenced object as a parameter into a function?

Question

Here is a super quick example of what I am thinking.

// Example program
#include <iostream>
#include <string>

class Obj
{
private:
    int myInt;
public:
    Obj()
    {
        myInt = 0;
    }
    ~Obj()
    {}
    
    void increment()
    { 
        myInt++; 
    }
    int getInt() 
    { 
        return myInt; 
    }
};

class A
{
public:
    Obj* myObj;
    A()
    {
        myObj = nullptr;
    }
    ~A()
    {
        if(myObj)
        {
            delete myObj;
            myObj = nullptr;
        }
    };
    
    void myFunc(Obj* o)
    {
        myObj = o;
    }
};

int main()
{
    A* a = new A();
    a->myFunc(new Obj());
    a->myObj->increment();
    delete a;
}

Just as a hypothetical .... regarding the above code - specifically the line

a->myFunc(new Obj());

It compiles fine, but is there anything functionally wrong with doing this? Other than maybe poor form, or going against best practices?

There is not a memory leak. There is a Rule of Three violation . Stylistically, smart pointers are preferred as they can convey the transfer of ownership. — Drew Dormann, Feb 25 '22 at 19:59
Unrelated: Take advantage of the [Member Initializer List](https://en.cppreference.com/w/cpp/language/constructor) where you can. It won't help much here, default initialization of a pointer is to do nothing, but all members must be initialized before entering the body of the constructor, and if the initialization is expensive and promptly followed with an assignment, you have increased the constructor's workload. — user4581301, Feb 25 '22 at 20:07
your code would be much more "ok" if you would remove use of any `new` — 463035818_is_not_an_ai, Feb 25 '22 at 20:09
FYI, `delete` can safely be called on a `nullptr`, so you don't need to check for that condition explicitly. — Remy Lebeau, Feb 25 '22 at 20:11
the code has no error, to say if it is "ok" you need to explain what it is good for. How is it better than `int x; ++x` ? Anyhow, for review of working code there is https://codereview.stackexchange.com/ — 463035818_is_not_an_ai, Feb 25 '22 at 20:17
https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — 463035818_is_not_an_ai, Feb 25 '22 at 20:18
You can also hide the creation of the actual pointer in `A` unless there's a reason why the users of `A` should need to know how `A` stores its interal resources. [example](https://godbolt.org/z/5dGxjd8Y3) (building on @WhozCraig's example) — Ted Lyngmo, Feb 25 '22 at 20:23

score 1 · Answer 1 · answered Feb 25 '22 at 20:18

An important problem with your code is that it's very fragile and bug-prone.

For example, what happens if you have two instances of class A, that point to the same Obj instance?

Suppose that one of the A instances is deleted. Then the object pointed by myObj is destructed. But now the other instance of A has a raw pointer that is pointing to a memory that is not valid anymore.

A better way of storing object pointers, assuming shared ownership semantic, is to use a smart pointer like std::shared_ptr.

For example, in class A replace the raw pointer Obj* myObj with a smart pointer like std::shared_ptr<Obj>, and don't explicitly invoke delete myObj from the A's destructor. Instead, let the std::shared_ptr's destructor automatically handle the deletion of the smart pointer data member. What will happen under the hood is that shared_ptr will use a reference count to figure out when there are no more references to the pointed object, and when this reference count goes to zero, the pointed object will be deleted.

Is it OK to pass an unreferenced object as a parameter into a function?

1 Answers1