0
\$\begingroup\$

I'm doing the following exercise from PPP - Bjarne Stroustrup, Chapter 19, ex.10.

Implement a simple unique_ptr supporting only a constructor, destructor,->, * and release(). In particular, don't try to implement an assignment or a copy constructor

Here's what I've done so far, for which I'd like to have a check. I did this without looking at available resources on this site or on the web, so I'd like to have a check and, of course, any suggestion in highly appreciated! I tried to copy the declarations from the std::unique_ptr reference on the web.

#include <iostream>
#include <vector>

template <typename T>
class Unique_Ptr
{
private:
    T *ptr;

public:
    explicit Unique_Ptr(T *p) noexcept : ptr{p} {}

    ~Unique_Ptr() { delete ptr; };

    T *operator->() const noexcept { return ptr; }

    inline T operator*() const { return *ptr; }

    inline T *get() const noexcept { return ptr; }

    T *release() noexcept
    {
        auto tmp = ptr;
        ptr = nullptr;
        return tmp;
    }
};

int main()
{
    //Constructor test
    Unique_Ptr p{new int{2}};
    std::cout << *p << "\n";

    //Test with pointer to a vector
    Unique_Ptr pp{new std::vector<int>{1, 2, 3}};
    std::cout << pp->size() << "\n";

    //release() test
    auto test_release = pp.release();
    std::cout << (pp.get() == nullptr) << "\n"; //expect 1

    std::cout << test_release->size() << "\n";

    return 0;
}
\$\endgroup\$
3
  • \$\begingroup\$ You need to deal with copy / move constructors and assignments - their implementation and declarations are missing. Also default one ought to set ptr to nullptr. \$\endgroup\$
    – ALX23z
    Sep 13 '21 at 13:08
  • \$\begingroup\$ I have rolled back Rev 3 → 1. Please see What to do when someone answers. \$\endgroup\$ Sep 13 '21 at 17:25
  • 1
    \$\begingroup\$ Ok, I'm going to ask a new question! Thanks. \$\endgroup\$ Sep 13 '21 at 17:26
1
\$\begingroup\$

First of all, you don't need to write inline when a function is defined inside the class. You might notice that you've used it in some but not all of them; it doesn't add anything to the meaning.

In particular, don't try to implement an assignment [operator] or a copy constructor

(N.B. did you leave out the word operator when you typed the quotation?)

Rather than leaving them off, which will cause an incorrect default implementation to be generated, mark them as =delete. That way you'll get an error if your test code would try to make use of them, rather than just some bizarre malfunction. Use the "rule of 5".

T *operator->()
That looks odd...
The style in C++ is to put the * or & with the type, not the identifier. This is called out specifically near the beginning of Stroustrup’s first book, and is an intentional difference from C style.

The problem did specify a constructor implying the singular. And you've given one, which prevents a default constructor from being automatically generated. But you could add a default constructor or add a default argument to your one-argument constructor, without getting into territory that's too advanced for you.

std::cout << test_release->size() << "\n";
This should cause a run-time error when it dereferences the nullptr. That's not a good way to implement a test program, especially without commenting that.

BTW, using '\n' instead of "\n" is more efficient.

All in all, it looks good.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks for your answer. I updated my question with the fixed code. I need, however, a check on my move semantics. I wrote in the edit what's my issue \$\endgroup\$ Sep 13 '21 at 17:19
  • \$\begingroup\$ I've posted a new question here with the improved version you suggested: codereview.stackexchange.com/questions/267976/… \$\endgroup\$ Sep 13 '21 at 17:31
  • 1
    \$\begingroup\$ operator->() should also return a T&, not a T*. \$\endgroup\$
    – G. Sliepen
    Sep 13 '21 at 20:41
0
\$\begingroup\$
  1. The default constructor is missing.

  2. The move constructor and move assignment operator are missing (they are not copying!).

Otherwise, it looks good.

Although a matter of style, I find that it helps to indicate pointer types even when using auto, i.e.

auto* test_release = pp.release();

When writing "ad hoc" tests, prefer to use assert instead of writing to the console. You don't need human eyes to verify that the results are correct:

constexpr auto N = std::vector<int>::size_type(3);

auto *const vector = new std::vector<int>(N);
std::iota(vector->begin(), vector->end(), 0);
assert(vector->size() == N);

Unique_Ptr pp{vector};
assert(pp->get() == vector);
assert(pp->size() == N);

auto *test_release = pp.release();
assert(!pp->get());
assert(test_release == vector);
assert(test_release->size() == N);

Such tests are then very easy to convert to use an actual testing framework. E.g. for the Catch2 framework, just replace assert with REQUIRE or CHECK (those two are equivalent, the latter continues testing even if the statement wasn't true).

Output to console may be useful when debugging, but even then examining the state in the debugger may save time.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your useful answer. I've updated my code using your suggestions about assert. \$\endgroup\$ Sep 13 '21 at 17:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.