If post
is not going to outlive this
, and is always going to be a child of this
(meaning it will never get reparented) then you can store the instances in a vector
of unique_ptr<Post>
:
class This: /* ... */ {
// ...
std::vector<std::unique_ptr<Post>> post_vec_;
};
Now your code can omit the deletion:
auto post = std::make_unique<Post>(this);
QString url;
ParseResult result = parse(url, post.get());
if (result == ParseResult::Success) {
post_vec_.push_back(std::move(post));
// 'post' was moved from, so it's just a nullptr now.
}
// No 'else' needed. 'post' is going to be deleted when it goes out of scope.
(This assumes that post->deleteLater()
could have been replaced with delete post
in your original code. It doesn't seem like you have a reason to use deleteLater()
rather than just calling delete
directly.)
When this
is destroyed, its members are going to be destroyed first before calling the base class destructor. So post_vec_
is going to get destroyed first, which means the smart pointers it contains are going to delete the Post
objects they manage. The destructors of those objects are going to unregister the objects from their parents, so no double-deletion can happen. This is why this answer has the requirement that these objects must not outlive this
and must not get reparented.
As a final note, if you don't want to expose the Post
class in the This
header, you can just forward declare it:
class Post;
class This: /* ... */ {
// ...
std::vector<std::unique_ptr<Post>> post_vec_;
};
unique_ptr
can still work with that, as long as you implement your destructor in the .cpp
file. If you don't have a destructor, then you can use the default one, but you need to default it in the .cpp
file:
class Post;
class This: /* ... */ {
public:
// ...
~This() override;
// ...
std::vector<std::unique_ptr<Post>> post_vec_;
};
In the .cpp
file:
This::~This() = default;