Search code examples
c++memory-managementshared-ptropengl-2.0

Class design: how to return a shared_ptr: reference or copy


This is the scenario: I have a class named Program, which holds three shared_ptr: to the vertex, geometry and fragment shader. When a Shader object gets constructed, it creates the shader with glCreateShader, and it also compiles it.

The Shader denstructor automatically calls glDeleteShader. So the problem is that if I do the following:

  1. Create a shader object;
  2. Copy it;
  3. Destroy the copy.

Also the original copy gets invalidated, because when the copy is destroyed it calls glDeleteShader. That's a design problem, I believe.

So I avoided this problem by just using pointers. Now the Program class holds the shaders. I created a method that returns the shared_ptr to the vertex, geometry and fragment Shader objects. My doubt is: should I return the shared_ptr like this:

const shared_ptr<Shader>& getVertex_shader_ptr() const
{
    return vertex_shader_ptr;
}

Or like this:

shared_ptr<Shader> getVertex_shader_ptr() const
{
    return vertex_shader_ptr;
}

My fear is that the problem that I described above occurs again: the shared_ptr gets deallocated and it invalidates the OpenGL shader.


Solution

  • Unless it's valid for your shader to be NULL, you should just return a reference to it:

    const Shader& getVertex_shader() const
    {
        return vertex_shader;
    }
    

    If it's valid for your shader to be NULL, but only your program is responsible for deleting it, just return a pointer:

    const Shader* getVertex_shader_ptr() const ...
    

    The semantics of a shared pointer is for when there's no clear ownership of the shader. If that's the case, then return by value. The semantics are that two pieces of the program have an interest in the object not being cleaned up, and they both need a way to keep it alive.

    shared_ptr<Shader> getVertex_shader_ptr() const ...