Ошибка конструктора копирования при вставке элементов в стек

Question

Мне дали класс Node и Stack в моем .h файле. Мне нужно реализовать конструктор копирования, оператор присваивания и деструктор и протестировать их в другом тестовом файле. При тестировании конструктора копирования после вставки 3 элементов он отображает только один элемент. Я не знаю, что случилось; вот мой файл .h для справки:

#ifndef _STACK_H
#define _STACK_H
#include <iostream>
#include <exception>
using std::ostream;
using std::cout;
using std::endl;
using std::range_error;
// Forward declarations
template <class T> class Stack;
template <class T> class Node;
template <class T> ostream& operator<<(ostream&, const Node<T>&);

// Node class for linked list
template <class T>
class Node {
    friend Stack<T>;
public:
    Node(T data = T(), Node<T>* next = nullptr) {
        _data = data;
        _next = next;
    }
    friend ostream& operator<< <T>(ostream& sout, const Node<T>& x);
private:
    T _data;
    Node* _next;
};
// Overloaded insertion operator.  Must be overloaded for the template
// class T, or this won't work!
template <class T>
ostream& operator<<(ostream& sout, const Node<T>& x) {
    sout << "Data: " << x._data;
    return sout;
}

// Stack class.  Linked-list implementation of a stack. Uses the Node
// class.
template <class T>
class Stack {
public:
    // Constructor
    Stack();

    // Copy constructor, assignment operator, and destructor
    // DO NOT IMPLEMENT HERE.  SEE BELOW.
    Stack(const Stack& rhs);
    const Stack& operator=(const Stack& rhs);
    ~Stack();

    void push(const T& data);
    const T& top() const;
    void pop();
    bool empty() const;  // Returns 'true' if stack is empty
    void dump() const;

    //Delete method used for destructor
    void nullify();

private:
    Node<T>* _head;
    Node<T>* _temp1;
    Node<T>* _temp2; //pointers
};

template <class T>
Stack<T>::Stack() {
    _head = nullptr;
}

template <class T>
Stack<T>::Stack(const Stack<T>& rhs) {
    if (rhs._head != nullptr) {
        _head = new Node<T>(rhs._head->_data);
        _temp1 = _head->_next;  //temp1 would be the next one after head
        //_temp2 = _temp2->_next; 
        while (_temp2 != nullptr) {
            _temp1 = new Node<T>(_temp2->_data);
            _temp1 = _temp1->_next;
            _temp2 = _temp2->_next; //temp2 should be the next node after temp1
        }
    }
    else
        _head = nullptr;
}

template <class T>
const Stack<T>& Stack<T>::operator=(const Stack<T>& rhs) {
    if (this != &rhs) {
        nullify();
        if (rhs._head != nullptr) {
            _head = new Node<T>(rhs._head->_data);
            _temp1 = _head->_next;  //temp1 would be the next one after head
            //_temp2 = _temp2->_next; 

            while (_temp2 != nullptr) {
                _temp1 = new Node<T>(_temp2->_data);
                _temp1 = _temp1->_next;
                _temp2 = _temp2->_next; //temp2 should be the next node after temp1
            }
        }
        else
            _head = nullptr;
    }
    return *this;
}

template <class T>
Stack<T>::~Stack() {
    nullify();
}

template <class T>
void Stack<T>::nullify() {
    while (!empty()) {
        pop();
    }
}

template <class T>
void Stack<T>::pop() {
    if (empty()) {
        throw range_error("Stack<T>::pop(): attempt to pop from an empty stack.");
    }

    Node<T>* tmpPtr = _head->_next;
    delete _head;
    _head = tmpPtr;
}

template <class T>
bool Stack<T>::empty() const {
    return _head == nullptr;
}

template <class T>
void Stack<T>::push(const T& data) {
    Node<T>* tmpPtr = new Node<T>(data);
    tmpPtr->_next = _head;
    _head = tmpPtr;
}

template <class T>
const T& Stack<T>::top() const {
    if (empty()) {
        throw range_error("Stack<T>::top(): attempt to read empty stack.");
    }

    return _head->_data;
}

template <class T>
void Stack<T>::dump() const {
    Node<T>* nodePtr = _head;
    while (nodePtr != nullptr) {
        cout << nodePtr->_data << endl;
        nodePtr = nodePtr->_next;
    }
}
#endif

При нажатии 34, 67, 92 он показывает только 92 для конструктора копирования во время вывода. Вот код, для которого я тестирование моего .h кода:

#include "stack.h"
#include <iostream>
using namespace std;
using std::cout;
using std::endl;

int main()
{
    cout << "Testing default constructor\n";

    Stack<int> intStack;

    intStack.dump();
    cout << "Stack is empty initially\n\n";

    intStack.push(34);
    intStack.push(67);
    intStack.push(92);

    cout << "Testing copy constructor after inserting 92, 67 & 34: \n";
    Stack<int> test1(intStack);
    //cout << "Dumping intStack into Test1 & displaying it: \n";
    test1.dump();

    cout << "\nTesting destructor: \n";
    test1.nullify();
    test1.dump();
    cout << "Its empty\n\n";

    Stack<int> test2;

    test2.push(75);
    test2.push(56);
    test2.push(88);
    test2.push(69);

    cout << "Testing assignment operator after inserting 69, 88, 56 & 75: \n";

    Stack<int> test3;

    test3 = test2;
    test3.dump();

    cout << "\nTesting destructor: \n";
    test2.nullify();
    test2.dump();
    cout << "Its empty\n\n";

    return 0;
}

Я до сих пор не полностью привык к C ++, поэтому извиняюсь за любые ошибки.

Answer 1

В вашем классе Stack есть несколько ошибок.

Во-первых, конструктор копирования не инициализирует все члены, как и ваш конструктор по умолчанию. Их необходимо исправить:

template <class T>
Stack<T>::Stack() : _head(nullptr), _temp1(nullptr), _temp2(nullptr) {}

template <class T>
Stack<T>::Stack(const Stack<T>& rhs) : _head(nullptr), _temp1(nullptr), _temp2(nullptr)
{
  //...
}

Как только это будет сделано, конструктор копирования может быть легко реализован с помощью другой существующей функции Stack::push. Ваша реализация слишком сложна.

template <class T>
Stack<T>::Stack(const Stack<T>& rhs) : _head(nullptr), _temp1(nullptr), _temp2(nullptr) {
    Node<T>* temp = rhs._head;
    while (temp)
    {
        push(temp->_data);
        temp = temp->_next;
    }
}

Что здесь делается? Просто - все, что мы делаем, это берем заголовок переданного Stack и перебираем элементы, вызывающие Stack::push, чтобы добавить данные в новый объект Stack. Поскольку у вас уже закодирована функция push, вы должны ее использовать.

Во-вторых, обратите внимание, что мы используем локальную переменную temp. Я сомневаюсь, что вам нужен какой-либо из этих _temp членов в вашем классе, но это другая история.

Наконец, ваш оператор присваивания можно легко реализовать, если у вас есть конструктор копирования и деструктор для Stack :

template <class T>
const Stack<T>& Stack<T>::operator=(const Stack<T>& rhs) {
    if (this != &rhs) {
        Stack<T> temp = rhs;
        std::swap(temp._head, _head);
        std::swap(temp._temp1, _temp1);
        std::swap(temp._temp2, _temp2);
    }
    return *this;
}

Этот метод использует копирование / свопинг . Все, что делается, - это создать временный объект из переданного объекта Stack и просто заменить текущее содержимое содержимым временного. Затем временное отмирает со старым содержимым.

Учитывая все это, класс должен работать правильно. Правильно ли это на 100% со всеми другими функциями, это опять же другой вопрос.

Изменить:

Вот исправление для конструктора копирования. Обратите внимание, что мы по-прежнему используем существующие функции для создания копии:

  template <class T>
    Stack<T>::Stack(const Stack<T>& rhs) : _head(nullptr), _temp1(nullptr), _temp2(nullptr) {
        Node<T>* temp = rhs._head;
        Stack<T> tempStack;
        while (temp)
        {
            tempStack.push(temp->_data);
            temp = temp->_next;
        }
        while (!tempStack.empty())
        {
           push(tempStack.top()); 
           tempStack.pop();
        }
    }

Это не так эффективно, но обычно структура данных стека использует базовый контейнер, такой как vector, где легко отменить базовое содержимое , а не на основе односвязного списка, который вы используете.