Skip to main content
added 58 characters in body
Source Link

I would use unique_ptr to manage Node lifetimes, one caveat here is performance may be slightly suboptimal on Stack destruction due to the recursive destructor calls. I'd also make Node a private nested struct in Stack since it's an implementation detail. Since t is a sink parameter to the Node constructor I'dyou could take it by value and std::move it into place but since you're the only one using the Node constructor you can save a possible extra move by making the parameter T&& t.

If you use unique_ptr to manage Node lifetimes and C++11 member initialization for n you can make your default constructor =default= default.

Stack::push() should take its argument by value for the same reason as the Node construtor, it's a sink argument. In this case you could avoid a possible extra move by providing both push(const T&) and push(T&&) overloads but taking by value is simpler.

I would use unique_ptr to manage Node lifetimes, one caveat here is performance may be slightly suboptimal on Stack destruction due to the recursive destructor calls. I'd also make Node a private nested struct in Stack since it's an implementation detail. Since t is a sink parameter to the Node constructor I'd take it by value and std::move it into place.

If you use unique_ptr to manage Node lifetimes and C++11 member initialization you can make your default constructor =default.

Stack::push() should take its argument by value for the same reason as the Node construtor, it's a sink argument.

I would use unique_ptr to manage Node lifetimes, one caveat here is performance may be slightly suboptimal on Stack destruction due to the recursive destructor calls. I'd also make Node a private nested struct in Stack since it's an implementation detail. Since t is a sink parameter to the Node constructor you could take it by value and std::move it into place but since you're the only one using the Node constructor you can save a possible extra move by making the parameter T&& t.

If you use unique_ptr to manage Node lifetimes and C++11 member initialization for n you can make your default constructor = default.

Stack::push() should take its argument by value for the same reason as the Node construtor, it's a sink argument. In this case you could avoid a possible extra move by providing both push(const T&) and push(T&&) overloads but taking by value is simpler.

added 58 characters in body
Source Link

If you use unique_ptr to manage Node lifetimes you don't need an explicit destructor, the default destructor will be correct. The default move constructor and move assignment can also be used, although you must explicitly request them. Copy assignment can be implemented using 'copy and swap'.

template <typename T>
class Stack {
    struct Node {
        T item;
        std::unique_ptr<Node> next;
        Node(TT&& t, std::unique_ptr<Node>&& tail) : item{std::move(t)}, next{std::move(tail)} {}
    };

public:
    Stack() = default;
    Stack(const Stack&);
    Stack(Stack&& x) = default;
    Stack& operator=(const Stack& x) {
        Stack temp{x};
        std::swap(*this, temp);
        return *this;
    }
    Stack& operator=(Stack&&) = default;

    auto size() const noexcept { return n; }
    bool empty() const noexcept { return n == 0; }

    void push(T t) {
        head = std::make_unique<Node>(std::move(t), std::move(head));
        ++n;
    }
    void pop() noexcept {
        head = std::move(head->next);
        --n;
    }

    T& top() noexcept { return head->item; }
    const T& top() const noexcept { return head->item; }

private:
    std::unique_ptr<Node> head;
    std::size_t n{};
};

template <typename T>
Stack<T>::Stack(const Stack& s) {
    Stack<T> temp;
    auto it = s.head.get();
    while (it) {
        temp.push(it->item);
        it = it->next.get();
    }
    while (!temp.empty()) {
        push(std::move(temp.top()));
        temp.pop();
    }
}

If you use unique_ptr to manage Node lifetimes you don't need an explicit destructor, the default destructor will be correct. The default move constructor and move assignment can also be used, although you must explicitly request them.

template <typename T>
class Stack {
    struct Node {
        T item;
        std::unique_ptr<Node> next;
        Node(T t, std::unique_ptr<Node>&& tail) : item{std::move(t)}, next{std::move(tail)} {}
    };

public:
    Stack() = default;
    Stack(const Stack&);
    Stack(Stack&& x) = default;
    Stack& operator=(const Stack& x) {
        Stack temp{x};
        std::swap(*this, temp);
        return *this;
    }
    Stack& operator=(Stack&&) = default;

    auto size() const noexcept { return n; }
    bool empty() const noexcept { return n == 0; }

    void push(T t) {
        head = std::make_unique<Node>(std::move(t), std::move(head));
        ++n;
    }
    void pop() noexcept {
        head = std::move(head->next);
        --n;
    }

    T& top() noexcept { return head->item; }
    const T& top() const noexcept { return head->item; }

private:
    std::unique_ptr<Node> head;
    std::size_t n{};
};

template <typename T>
Stack<T>::Stack(const Stack& s) {
    Stack<T> temp;
    auto it = s.head.get();
    while (it) {
        temp.push(it->item);
        it = it->next.get();
    }
    while (!temp.empty()) {
        push(std::move(temp.top()));
        temp.pop();
    }
}

If you use unique_ptr to manage Node lifetimes you don't need an explicit destructor, the default destructor will be correct. The default move constructor and move assignment can also be used, although you must explicitly request them. Copy assignment can be implemented using 'copy and swap'.

template <typename T>
class Stack {
    struct Node {
        T item;
        std::unique_ptr<Node> next;
        Node(T&& t, std::unique_ptr<Node>&& tail) : item{std::move(t)}, next{std::move(tail)} {}
    };

public:
    Stack() = default;
    Stack(const Stack&);
    Stack(Stack&& x) = default;
    Stack& operator=(const Stack& x) {
        Stack temp{x};
        std::swap(*this, temp);
        return *this;
    }
    Stack& operator=(Stack&&) = default;

    auto size() const noexcept { return n; }
    bool empty() const noexcept { return n == 0; }

    void push(T t) {
        head = std::make_unique<Node>(std::move(t), std::move(head));
        ++n;
    }
    void pop() noexcept {
        head = std::move(head->next);
        --n;
    }

    T& top() noexcept { return head->item; }
    const T& top() const noexcept { return head->item; }

private:
    std::unique_ptr<Node> head;
    std::size_t n{};
};

template <typename T>
Stack<T>::Stack(const Stack& s) {
    Stack<T> temp;
    auto it = s.head.get();
    while (it) {
        temp.push(it->item);
        it = it->next.get();
    }
    while (!temp.empty()) {
        push(std::move(temp.top()));
        temp.pop();
    }
}
added 179 characters in body
Source Link

I'd call Stack::peek() Stack::top() for consistency with std::stack. It should also return a reference rather than by value. This is both for efficiency (no copies of the item) and also avoids the possibility of top() throwing an exception (T's copy constructor might throw an exception in general).

If you use unique_ptr to manage Node lifetimes you don't need aan explicit destructor, the default destructor will be correct. The default move constructor and move assignment can also be used, although you must explicitly request them.

Here's how I'dI might implement it:

template <class<typename T>
class Stack {
    struct Node {
        T item;
        std::unique_ptr<Node> next;
        Node(T t, std::unique_ptr<Node>&& tail) : item{std::move(t)}, next{std::move(tail)} {}
    };

public:
    Stack() = default;
    Stack(const Stack&);
    Stack(Stack&& x) = default;
    Stack& operator=(const Stack& x) {
        Stack temp{x};
        std::swap(*this, temp);
        return *this;
    }
    Stack& operator=(Stack&&) = default;

    auto size() const noexcept { return n; }
    bool empty() const noexcept { return n == 0; }

    void push(T t) {
        head = std::make_unique<Node>(std::move(t), std::move(head));
        ++n;
    }
    void pop() noexcept {
        head = std::move(head->next);
        --n;
    }

    T& top() noexcept { return head->item; }
    const T& top() const noexcept { return head->item; }

private:
    std::unique_ptr<Node> head;
    std::size_t n{};
};

template <class<typename T>
Stack<T>::Stack(const Stack& s) {
    Stack<T> temp;
    auto it = s.head.get();
    while (it) {
        temp.push(it->item);
        it = it->next.get();
    }
    while (!temp.empty()) {
        push(std::move(temp.top()));
        temp.pop();
    }
}

I'd call Stack::peek() Stack::top() for consistency with std::stack. It should also return a reference rather than by value.

If you use unique_ptr to manage Node lifetimes you don't need a destructor.

Here's how I'd implement it:

template <class T>
class Stack {
    struct Node {
        T item;
        std::unique_ptr<Node> next;
        Node(T t, std::unique_ptr<Node>&& tail) : item{std::move(t)}, next{std::move(tail)} {}
    };

public:
    Stack() = default;
    Stack(const Stack&);
    Stack(Stack&& x) = default;
    Stack& operator=(const Stack& x) {
        Stack temp{x};
        std::swap(*this, temp);
        return *this;
    }
    Stack& operator=(Stack&&) = default;

    auto size() const { return n; }
    bool empty() const { return n == 0; }

    void push(T t) {
        head = std::make_unique<Node>(std::move(t), std::move(head));
        ++n;
    }
    void pop() {
        head = std::move(head->next);
        --n;
    }

    T& top() { return head->item; }
    const T& top() const { return head->item; }

private:
    std::unique_ptr<Node> head;
    std::size_t n{};
};

template <class T>
Stack<T>::Stack(const Stack& s) {
    Stack<T> temp;
    auto it = s.head.get();
    while (it) {
        temp.push(it->item);
        it = it->next.get();
    }
    while (!temp.empty()) {
        push(std::move(temp.top()));
        temp.pop();
    }
}

I'd call Stack::peek() Stack::top() for consistency with std::stack. It should also return a reference rather than by value. This is both for efficiency (no copies of the item) and also avoids the possibility of top() throwing an exception (T's copy constructor might throw an exception in general).

If you use unique_ptr to manage Node lifetimes you don't need an explicit destructor, the default destructor will be correct. The default move constructor and move assignment can also be used, although you must explicitly request them.

Here's how I might implement it:

template <typename T>
class Stack {
    struct Node {
        T item;
        std::unique_ptr<Node> next;
        Node(T t, std::unique_ptr<Node>&& tail) : item{std::move(t)}, next{std::move(tail)} {}
    };

public:
    Stack() = default;
    Stack(const Stack&);
    Stack(Stack&& x) = default;
    Stack& operator=(const Stack& x) {
        Stack temp{x};
        std::swap(*this, temp);
        return *this;
    }
    Stack& operator=(Stack&&) = default;

    auto size() const noexcept { return n; }
    bool empty() const noexcept { return n == 0; }

    void push(T t) {
        head = std::make_unique<Node>(std::move(t), std::move(head));
        ++n;
    }
    void pop() noexcept {
        head = std::move(head->next);
        --n;
    }

    T& top() noexcept { return head->item; }
    const T& top() const noexcept { return head->item; }

private:
    std::unique_ptr<Node> head;
    std::size_t n{};
};

template <typename T>
Stack<T>::Stack(const Stack& s) {
    Stack<T> temp;
    auto it = s.head.get();
    while (it) {
        temp.push(it->item);
        it = it->next.get();
    }
    while (!temp.empty()) {
        push(std::move(temp.top()));
        temp.pop();
    }
}
added 1323 characters in body
Source Link
Loading
deleted 34 characters in body
Source Link
Loading
added 1176 characters in body
Source Link
Loading
Source Link
Loading