Моя программа непрерывно зацикливается после чтения узлов в связанный список и отображает только один узел повторно. Может кто-нибудь помочь мне исправить это?

Question

Я создал Связанный список в программе C ++, которая записывает узлы в двоичный файл и считывает их обратно в связанный список.Однако при отображении списка обратно программа непрерывно отображает только один узел (непрерывный цикл).Кто-нибудь поможет мне решить эту проблему, пожалуйста?Вот код:

 #ifndef ACCOUNT_H
    #define ACCOUNT_H

    class Account{

private:
    double balance;
    unsigned int accountNumber;
    Account *next;
public:
    Account(){
        balance=0.0;
        accountNumber=0;
        next=NULL;
    }// primary constructor

    Account(unsigned int acc, double bal){
        accountNumber=acc;
        balance=bal;
        next=NULL;
    }// primary constructor

    Account::~Account(){
        //delete next;
    }// destructor


    void setBalance(double b){
        balance=b;
    }// end of setBalance


    double getBalance(){
        return balance;
    }

    void setAcc(unsigned int acc){
        accountNumber=acc;
    }

    unsigned int getAccNum(){
        return accountNumber;
    }

    //  links
    void setNextAccount(Account *nAcc){
        next=nAcc;
        //delete nAcc;
    }// Mutator for next

    Account *getNextAccount(){
        //Account *temp=next;
        return next;
    }
};
#endif

Заголовочный файл связанного списка:

#ifndef ACCOUNTLIST_H
#define ACCOUNTLIST_H

class AccountList{
private :
    Account *head;
    //Account * temp;
    //Account *current;
public:
    AccountList(){
        head=NULL;
        //temp=NULL;
        //current=NULL;
        //current=NULL;
    }// end of default constructor

    AccountList::~AccountList(){
        /*Account *temp;
        while (head!= NULL)
        {
            temp = head;
            head = head->getNextAccount();
            delete temp;
        } */
        delete head;
        //delete current;
    }// destructor for list

    void addNode(Account *h){
        //Account *temp;
        Account *current;
        //temp=h;
        //temp->setNextAccount(NULL);
        if(head==NULL){
            head=h;
        }
        else{
            current=head;
            while((current->getNextAccount())!=NULL){
                current= current->getNextAccount();
            }
            current->setNextAccount(h);
        }
        //delete current;
    }// mutator for head

    void displayAll(){
        Account *temp=head;
        while ((temp!=NULL) && (temp->getAccNum()!=0)){         
            cout << "Account number: " <<temp->getAccNum() << " has a balnce of: " << temp->getBalance() <<endl;
            temp=temp->getNextAccount();
        }

        delete temp;
    }

    void displayNode(int id){
        Account *temp= head;
        while (temp !=NULL){

            if (temp->getAccNum()==id){
                //temp->display();
                cout << "Account Number : " << temp->getAccNum() <<"has a balance of " << temp->getBalance() <<endl;
                delete temp;
                return;
            }
            temp= temp->getNextAccount();// gets next node in the list
        }
        cout << "Employer was not found" << endl;
    }// end of displayNode


    Account* getHead(){
        return head;
    }

    void removeAll(){
        Account *temp;
        while (head!=NULL){
            temp=head;
            head= head->getNextAccount();
            delete temp;
        }
    }// end of method removeAll

};

#endif

основной драйвер:

#include <iostream>
#include <fstream>
#include "Account.h"
#Include "AccountList"
//#include <cstdlib>
#include <stdlib.h>
using namespace std;

void main(){
    Account *acc1=new Account(1, 546.34); // create object and initialize attributes 
    Account *acc2=new Account(2,7896.34);
    Account *acc3=new Account();
    AccountList *list1= new AccountList();
    AccountList *list2= new AccountList();

    // add nodes to linked list
    list1->addNode(acc1);
    list1->addNode(acc2);
    //cout <<"Hello"<<endl;

    // file operation
    ofstream outAccount("account.dat", ios::out|ios::binary);

    // checks if ofstream could open file
    if(!outAccount){
        cerr<<"File could not be open" << endl;
        exit(1);
    }   
    acc3=list1->getHead();
    while( acc3!=NULL){
        outAccount.write(reinterpret_cast < const char*>(&acc3), sizeof(Account));
        acc3=acc3->getNextAccount();
        //outAccount.write(reinterpret_cast < const char*>(&acc2), `sizeof(Account));`
    }
    //cout <<"Hello"<<endl;
        outAccount.close();


    // read and display contents of file
    ifstream inAccount("account.dat", ios::in|ios::binary);
    if(!inAccount){
        cerr <<"File could not be openned for reading from file" << endl;
        exit(1);
    }
    //Account *accTemp=new Account();
    while(inAccount && !inAccount.eof()){

        inAccount.read(reinterpret_cast < char* >(&acc3), sizeof(Account));
        list2->addNode(acc3);

        //cout <<"Account Number : " << acc3->getAccNum()<< "has a balance of: " `<< acc3->getBalance() <<endl;`
    }

    inAccount.close();
    cout <<"Hello"<<endl;

    list2->displayAll();
    system("PAUSE");

    system("PAUSE");
}// end of main

Answer 1

Есть много проблем с вашим кодом, но я постараюсь охватить их все, не выкидывая исправленный список кода, и, надеюсь, вы сможете решить проблемы, основываясь на моих описаниях.

Прежде всего,нет необходимости заново изобретать «колесо» связанного списка, так как std :: list даст вам все, что вы пытаетесь сделать с вашим классом AccountList, но для этого вам нужно познакомиться с итераторами.По сути, итераторы - это то же самое, что и указатели, но я допускаю, что они могут сбить с толку тех, кто имеет опыт работы с Си.В любом случае, оставшаяся часть моего обсуждения предполагает, что вы продолжаете использовать свой класс AccountList вместо использования std :: list.

Во-вторых, в ваших конструкторах Account и AccountList вы должны использовать списки инициализатора для инициализации вашегопеременные-члены, так как нет ничего о переменных, которые требуют каких-либо вычислений или сложной логики.Например, сделайте следующее:

Account()
 : balance(0.0), accountNumber(0), next(NULL) {}

Теперь перейдем к фактическим ошибкам:

Когда вы выполняете outAccount.write (), вы пишете все содержимое acc3, включая его указатели.Эти указатели действительны только в этот момент и только для «list1».В следующий раз, когда вы запустите вашу программу, система может выделить что-то еще по этим адресам, и эти указатели не будут содержать объекты «Учетная запись» намного меньше тех же, что и раньше.Это важно понимать, когда вы переходите к inAccount.read (), где вы читаете старое содержимое объектов Account вместе со старыми значениями указателя.На данный момент эти адреса больше не действительны или, по крайней мере, не относятся к «list2».Это важно понимать, когда вы вызываете list2-> addNode (acc3).Теперь посмотрите на AccountList :: addNode ().Объект Account, который вы передаете, 'h', все еще содержит это старое значение указателя в своем поле 'next'.Ничто в коде, который читает в объекте Account, никогда не устанавливает его «next» в NULL, равно как и addNode ().Если вы не решите использовать std :: list <>, я бы порекомендовал вам запустить addNode (), вызвав h-> setNextAccount (NULL), прежде чем делать что-либо еще.Это устраняет устаревшую ошибку указателя.

Прежде чем перейти к следующей ошибке, я хотел бы упомянуть, что AccountList :: addNode () является временной сложностью O (n).При увеличении размера вашего AccountList сканирование займет все больше и больше времени до конца списка, чтобы просто добавить следующий объект Account в список.Вы можете сделать лучше: если вы настаиваете на добавлении следующей учетной записи в конец AccountList, то сохраняйте указатель в AccountList не только для заголовка, но и для хвостового узла.Таким образом, addNode () будет иметь временную сложность O (1).В качестве альтернативы, добавьте новый объект Account в начало списка: установите новый объект «next» для текущего заголовка, затем измените «head», чтобы быть вновь добавленным Account (таким образом, значительно упрощается логика для добавления нового объекта Account).к списку).Обратите внимание, что поиск учетной записи по номеру учетной записи по-прежнему будет осуществляться по порядку O (n), и если ваше программное обеспечение обрабатывает тысячи учетных записей, это довольно плохая производительность для поиска учетной записи.Вы можете получить O (log n) время поиска аккаунта, если используете std :: map.В этом случае ваше время «addNode» также будет O (log n), что хуже, чем O (1), но чем вы будете заниматься чаще, добавляя учетные записи или просматривая существующие учетные записи?Вероятно, поиск существующих учетных записей.

ОК, теперь следующая ошибка: в main (), когда вы проверяете условия выхода цикла inAccount.read () в первый раз, вы не читали даже в первомзапись.Думаю, ничего страшного, но когда вы, наконец, прочитали последний, inAccount.eof () еще не соответствует истине, поэтому вы снова вводите тело цикла, но на этот раз inAccount.read () ничего не читает.Так что acc3 не изменился с последней итерации.Затем вы по-прежнему вызываете list2-> addNode (acc3), даже если вы ничего не читали.Вы добавляете последнюю запись дважды!Я дам вам понять, как решить эту проблему.

Наконец, у вас происходит очень страшное управление памятью / объектами. Обратите внимание, что вы никогда не удаляете два динамически размещенных списка учетных записей в main (). Это не так уж важно, если вы намереваетесь, чтобы ваши AccountLists автоматически восстанавливались системой при выходе из вашей программы, но если вы хотите иметь несколько AccountLists, которые время от времени приходят и уходят в жизни вашей программы, вам нужно убедитесь, что они удаляют свое содержимое. Похоже, что вы хотите, чтобы ваши объекты AccountList владели назначенными им объектами Account, поскольку в вашем деструкторе ~ AccountList () вы удаляете «главный» объект Account. Поскольку деструктор ~ Account () не удаляет объект «next», единственным объектом Account в списке, который будет удален, является главный Account. Поскольку AccountList имеет только указатель на головной объект, возможно, допустимое место для удаления остальной части списка будет в ~ Account (), как я вижу, вы подумали о том, чтобы сделать это на основе закомментированного вызова удаления. Как вы и думали, каждый объект Account мог бы удалить только свое собственное «next», но в этом подходе есть небольшая ошибка: каждое последующее удаление добавляет еще один вызов функции в стек времени выполнения, рекурсивно вызывая delete для каждой учетной записи в список, пока не дойдет до конца списка. Если у вас есть тысячи объектов Account, вы гарантированно увеличите свой стек во время выполнения. Лучший способ сделать это будет следующим:

~Account(){
    Account *victim, *itsNext = next;
    while (itsNext){
        victim = itsNext;
        itsNext = victim->next;
        victim->next = NULL; // prevent recursion & blown stack
        delete victim;
    }
    next = NULL;
}

Хммм ... Я вижу, у вас есть метод AccountList :: removeAll (), который, кажется, структурирован очень похоже на мой ~ Account () выше. Возможно, вы можете просто вызвать это из своего деструктора ~ AccountList ().

Теперь, если вы решите использовать std :: list или std :: map (я бы порекомендовал map), вам не нужно беспокоиться об утечках памяти или о том, как / когда удалять, если ваш std :: list или std :: map - это список или карта фактических объектов Account, поскольку при удалении записи списка / карты (с помощью erase () или при удалении всего списка / карты) охватываемый объект (ы) Account удаляется прямо вместе с этим. Если они представляют собой список или карту указателей учетной записи, вам все равно придется просмотреть их и удалить их вручную. Если вы не использовали список auto_ptrs или smart_ptrs, но я отвлекся.

Answer 2

Здесь есть несколько проблем, вероятно, самая серьезная из которых - это способ, которым вы пытаетесь записать связанный список в файл.

Вы не можете записывать члены-указатели в файл и ожидать, что одни и те же адреса памяти будут содержать одни и те же объекты, когда вы вернетесь для чтения файла обратно. Вероятно, вам следует взглянуть на что-то вроде библиотеки boost serialization. или Буферы протокола Google .

Это отчасти связано с зависанием вашей программы; когда вы вызываете list2->addNode(acc3);, acc3 уже имеет значение для next, поэтому цикл while в addNode никогда не возвращается.

Кроме того, проверка !inAccount.eof() здесь бесполезна, поскольку вы не нажмете eof, пока не попытаетесь прочитать в третий раз. 3-е чтение завершается неудачно (ну, это читает 0 байтов), но вы все равно вызываете addNode. Вы можете проверить, сколько было прочитано, используя inAccount.gcount(). Если вы сделаете это сразу после чтения, и если это не ожидаемая сумма, то break вне цикла, вы избежите дополнительной попытки чтения.