How to Declare Std::Unique_Ptr and What Is the Use of It

Use of unique_ptr for class members

There is nothing technically wrong with your use of std::unique_ptr here, but it's totally unnecessary in this situation. std::vector is already essentially a unique pointer to an underlying array. Instead of having your class have a std::unique_ptr<std::vector<char>> member it should likely just have a std::vector<char> member.

---

I would also question why load_image and save_image are static if they need references/pointers to the class's members (or the object itself, in the case of save_image). It seems like it would make much more sense for them to be non-static member functions. That way they would implicitly have access to the members of the object on which they're called.

There's also a potential memory leak in your load_image function. You dynamically allocate the array pointed to by skip. If any of the read operations between there and when you delete skip were to throw an exception you would leak that array. 256 bytes may also not be enough to read the entire color map. You should probably use a std::vector<char>. Better yet, rather than reading the values you don't want, you could just seek past them:

// Skip the ID String
file_stream.seekg(header.id_length, std::ios::cur);

// Skip the colour map if it doesn't exist
if (!(tgaDesc & 4)) {
    int colourMapSize = header.colour_map_type * header.num_entries;
    file_stream.seekg(colourMapSize, std::ios::cur);
}

Writing that example made me notice that tgaDesc is always 0, so that if block will always run. Did you mean to check header.colour_map_type here? Of course, if there's no color map then header.num_entries should be 0, so I'm not sure the if is even needed at all.

While we're in load_image, you pass the std::ios::ate flag when opening file_stream but then immediately seekg back to the beginning of the file. If you remove the std::ios::ate flag then the stream will initially be positioned at the beginning of the file and the extra seekg could be eliminated.

The way you read the file header is mostly fine. Byte ordering (AKA endianness) could be a possible issue, but both TGA and most modern CPUs use little-endian byte order so you're probably fine unless you want to support some esoteric platform.

Putting that all together would give you an image class that looks something like this:

class image {
public:
    TGA_HEADER header;
    std::vector<char> pixel_data;

    image(const std::string& image_path);
    ~image();

    void save_image(const std::string& file_name);
    void load_image(const std::string& path);
};

image::image(const std::string& image_path)
{
    load_image(image_path);
}

void image::load_image(const std::string& path)
{
    std::ifstream file_stream(path, std::ios::in | std::ios::binary);

    if (file_stream.is_open()) {
        int tgaDesc = 0;

        /* read header */
        file_stream.read(&header.id_length, sizeof(header.id_length));
        file_stream.read(&header.colour_map_type, sizeof(header.colour_map_type));
        file_stream.read(&header.image_type, sizeof(header.image_type));

        file_stream.read((char*)(&header.first_entry), sizeof(header.first_entry));
        file_stream.read((char*)(&header.num_entries), sizeof(header.num_entries));
        file_stream.read(&header.bits_per_entry, sizeof(header.bits_per_entry));

        file_stream.read((char*)(&header.x_origin), sizeof(header.x_origin));
        file_stream.read((char*)(&header.y_origin), sizeof(header.y_origin));
        file_stream.read((char*)(&header.width), sizeof(header.width));
        file_stream.read((char*)(&header.height), sizeof(header.height));
        file_stream.read(&header.bits_per_pixel, sizeof(header.bits_per_pixel));
        file_stream.read(&header.descriptor, sizeof(header.descriptor));

        // Skip the ID String
        file_stream.seekg(header.id_length, std::ios::cur);

        // Skip the colour map if it doesn't exist
        if (!(tgaDesc & 4)) {
            int colourMapSize = header.colour_map_type * header.num_entries;
            file_stream.seekg(colourMapSize, std::ios::cur);
        }

        int imageDataSize = header.width * header.height * (header.bits_per_pixel / 8);
        pixel_data.resize(imageDataSize);

        /* read image data */
        file_stream.read(pixel_data.data(), imageDataSize);
    }
}

How to define the operator= for a unique_ptr wrapper class?

Turns out, as @che.wang pointed out, I actually needed a Converting Assignment Constructor based on (2) from the unique_ptr reference like:

template<typename U> Unique(Unique<U>&& other) { pointer = std::move(other.pointer); }

unique_ptr and forward declaration: the proper way to code a factory function

When the compiler instantiates the destructor of std::unique_ptr<Foo>, the compiler must find Foo::~Foo() and call it. This means that Foo must be a complete type at the point where std::unique_ptr<Foo> is destroyed.

This code is fine:

struct Foo;

std::unique_ptr<Foo> create();

...as long as you don't need to call the destructor of std::unique_ptr<Foo>! For a factory function that returns an std::unique_ptr to a class, that class needs to be a complete type. This is how you would declare the factory:

#include "foo.hpp"

std::unique_ptr<Foo> create();

You seem to be implementing pimpl with an std::unique_ptr correctly. You must define A::~A() at the point where B is complete (which is in the cpp file). You must define A::A() in the same place because B must be complete if you want to allocate memory and call its constructor.

So this is fine:

// a.hpp

struct A {
  A();
  ~A();

private:
  struct B;
  std::unique_ptr<B> b;
};

// a.cpp

struct A::B {
  // ...  
};

A::A()
  : b{std::make_unique<B>()} {}

A::~A() = default;

Now let's consider this (we'll pretend that I didn't make b private):

int main() {
  A a;
  auto b = std::move(a.b);
}

What exactly is going on here?

1. We are move constructing a std::unique_ptr<B> to initialize b.
2. b is a local variable which means that its destructor will be called at the end of the scope.
3. B must be a complete type when the destructor for std::unique_ptr<B> is instantiated.
4. B is an incomplete type so we cannot destroy b.

Ok, so you cannot pass around an std::unique_ptr<B> if B is an incomplete type. This restriction makes sense. pimpl means "pointer to implementation". It doesn't make sense for external code to access the implementation of A so A::b should be private. If you must access A::b then this is not pimpl, this is something else.

If you really must access A::b while keeping the definition of B hidden then there are a few workarounds.

std::shared_ptr<B>. This deletes the object polymorphically so that B doesn't need to be a complete type when the destructor of std::shared_ptr<B> is instantiated. It's not quite as fast as std::unique_ptr<B> and I personally prefer to avoid std::shared_ptr unless absolutely necessary.

std::unique_ptr<B, void(*)(B *)>. Similar to the way that std::shared_ptr<B> deletes the object. A function pointer is passed on construction that is responsible for deleting. This has the overhead of carrying around a function pointer unnecessarily.

std::unique_ptr<B, DeleteB>. The fastest possible solution. However, it's probably a little bit annoying if you have more than a handful of pimpl (but not really pimpl) classes because you can't define a template. This is how you would do it:

// a.hpp

struct DeleteB {
  void operator()(B *) const noexcept;
};

// a.cpp

void DeleteB::operator()(B *b) const noexcept {
  delete b;
}

Defining a custom deleter is probably the best option but if I were you, I'd find a way to avoid needing to access implementation details from outside the class.

What happens to a pointer after it has become a unique_ptr?

Wrapping a raw pointer inside of a smart pointer does not change anything about the raw pointer. It still points at the same memory it was originally pointing at. The smart pointer is merely copying the pointer and then managing the pointed-at memory for you, delete'ing the memory when the smart pointer is destroyed or is otherwise assigned a different pointer.

FYI, it would be more safer and more efficient to use std::make_unique() instead of new, eg:

template<typename T, typename... TArgs>
void addComponent(TArgs&&... MArgs) {
    if (!components.count(typeid(T))) {
        auto c = std::make_unique<T>(std::forward<TArgs>(MArgs)...);
        c->entity = this;
        c->init();
        components[typeid(T)] = std::move(c);
    }
}

What is the syntax to declare a unique_Ptr variable in a header, then assign it later in the constructor?

The following should work fine:

spriteBatch = std::unique_ptr<SpriteBatch>(new SpriteBatch(m_d3dContext.Get()));

Alternatively, you can avoid repeating the type name with some make_unique function.

spriteBatch = make_unique<SpriteBatch>(m_d3dContext.Get());

There's also the reset member:

spriteBatch.reset(new SpriteBatch(m_d3dContext.Get()));

But, since you mention a constructor, why not just use the member initialization list?

CubeRenderer::CubeRenderer()
: spriteBatch(new SpriteBatch(m_d3dContext.Get())) {}

Working with std::unique_ptr and std::queue

If I got your aim correctly, you definitely want

std::unique_ptr<MyObject::Packet> MyObject::FrontOfQueue()
{
  auto rv = std::move(PacketQueue.front());
  PacketQueue.pop();
  return rv;
}
// ...
std::unique_ptr<MyObject::Packet> frame = object.FrontOfQueue(); 

Notice, no raw pointers are used.

I think it will automatically deque once the address out of scope.

This assumption is wrong. Nothing is dequeued until .pop() is called.

---

Related Topics