Texture managing with smart pointers

I have an SDL_Wrapper which is working perfectly (it is not broken)! Please suggest how I could improve performance, and how I could make my unique_ptr(s) dispose automatically.

So far, I call my class like this:

CWindow window = std::make_unique("title", 0, 0, 140, 100, 0)

Is it the best way to do it? Will my unique_ptr dispose automatically?

Here's the SDL_Wrapper class:

```
#include "LogManager.h"
#include
#include

//SDL_Renderer wrapper

////SDL_Window wrapper !

class CWindowWrap
{

public:
CWindowWrap(const char* title, int xpos, int ypos, int width, int height, int flags)
: ptr_window(SDL_CreateWindow(title, xpos, ypos, width, height, flags))
{
LOG("SDL_Window Wrapper", "Constructed Window !");
}
virtual ~CWindowWrap()
{
if (ptr_window != nullptr){
SDL_DestroyWindow(ptr_window);
LOG("SDL_Window Wrapper", "Destroyed Window !");
}
else
LOG("SDL_Window Wrapper", "Window doesn't need to be destroyed !");
}

//disable copy constructor
CWindowWrap(CWindowWrap const&) = delete;
CWindowWrap& operator=(CWindowWrap const&) = delete;

//allow move
CWindowWrap(CWindowWrap&& move)
: ptr_window(nullptr)
{
using std::swap;
swap(ptr_window, move.ptr_window);
}

CWindowWrap& operator=(CWindowWrap&& move){
using std::swap;
swap(ptr_window, move.ptr_window);
}

operator SDL_Window() { return ptr_window; } // implicite conversion between CWindowWrap ---> SDL_Window;

private:
SDL_Window* ptr_window = nullptr;
};

typedef std::unique_ptr CWindow;

class CRendererWrap
{
public:

CRendererWrap(CWindowWrap window, int x, int y)
: ptr_renderer(SDL_CreateRenderer(window, x, y))
{}
~CRendererWrap()
{
if (ptr_renderer != nullptr){
SDL_DestroyRenderer(ptr_renderer);
}
}

//disable cop

I think that conceptually you're fine. There are some details that need attention.

Hide the implementation

Your creation syntax:

CWindow window = std::make_unique("title", 0, 0, 140, 100, 0)

requires the user to know that CWindow is a unique_ptr this isn't ideal. I believe that it's a good idea to hide the implementation class.

You should put your CWindowWrap class in an implementation namespace:

namespace detail{
    class CWindowWrap{
        ...
    };
}
using CWindow = std::unique_ptr;

Then you should provide a creator function like this:

CWindow make_window(const char* title, int xpos, int ypos, int width, int height, Uint32 flags){
     return std::make_unique(title, xpos, ypos, width, height, flags);
}

Don't use virtual if you don't need it

CWindowWrap doesn't need a virtual destructor as far as I can tell and I can't imagine why you would inherit from this. So simply remove virtual.

Wrapper classes should match types exactly

You're not matching the types properly for the constructor of CWindowWrap (flags should be Uint32 see here).

Move assignment/construction

As you have already initialized ptr_window in the in-class declaration, you do not need : ptr_window(nullptr) in your move constructor.

Your move assignment operator is missing a return statement.

That said, as you are not supposed to instantiate CWindowWrap directly you could simply = delete the move assignment operator and constructor. They won't be used.

Same comments goes for the other wrapper.

StackExchange Code Review Q#88015, answer score: 5