Constructor function reads from file and stores in linked list

Question

I want my program to retain its information within its linked list even after its ended. One way to do this is to read from a file and when done have the destructor save to that same file. I have the code to print to the file within the destructor, however its the reading from my file and storing that has me going nuts. Ive tried numerous ideas but i cant get it to work. Any one have any suggestions on what to add or fix? Beginner programmer here. I have seperate add, remove and clear functions applied.

The file im reading from looks like this:

HAHA
HEHE
MEME
OEL
URNE
WUWJ

I want to get each string into my linked list. Here is my code:

#pragma once
#include <string>
using namespace std;

class StringList;

class StringList
{
    private:
    struct StringListNode
    {
        StringListNode *pPrev;
        string          data;
        StringListNode *pNext;
    };
    public:
        StringList();
        ~StringList();
        void addToBottom(string s);
        void addToTop(string s);
        void remove(string s);
        void add(string s);
        string print();
        void clear();
        bool isEmpty() {return (pTop==NULL);}
    private:
        StringListNode * pTop;
        StringListNode * pBottom;
        StringListNode *find(const string &s);
};

The functions:

#include "StringList.h"
#include <string>
#include<iostream>
#include<fstream>

using namespace std;

StringList::StringList() 
{
   std::ifstream input( "Read.txt" );
   pTop = NULL;
   pBottom = NULL;
   for( std::string line; getline( input, line ); )
   {
      add(line);
   }
}


StringList::~StringList() {
    ofstream ofs("Read.txt");
    StringListNode *next;
    for (StringListNode *sp = pTop; sp != 0; sp = next)
    {
        ofs << sp->data << endl;
        next = sp->pNext;
        delete sp;
    }
}
void StringList::add(string s) //adds and places in alphabetical order
{
    if(pTop)
    {
        StringListNode *iter = pTop;
        while (iter && s > iter->data)
            iter = iter->pNext;

        if (iter)
        {
            // insert before
            StringListNode *A=new StringListNode;
            A->data = s;
            A->pNext = iter;
            A->pPrev = iter->pPrev;
            if (iter->pPrev)
                iter->pPrev->pNext = A;
            else
                pTop = A;
            iter->pPrev = A;
            return;
        }
    }

    // add to bottom
    addToBottom(s);
}

StringList::StringListNode *StringList::find(const string &s) //basic search function
{
    StringListNode *sp = pTop;   // Search
    while (sp != 0 && sp->data != s)
        sp = sp->pNext;
    return sp;
}


void StringList::addToTop(string s) //add to top of nodes
{
    if(isEmpty())
    {
        StringListNode * pNewNode;
        pNewNode = new StringListNode;
        (*pNewNode).data = s;
        pTop=pNewNode;
        pBottom=pNewNode;
        (*pNewNode).pPrev = NULL;
        (*pNewNode).pNext = NULL;
    }
    else //it's not empty
    {
        StringListNode * pNewNode;
        pNewNode = new StringListNode;
        (*pNewNode).data = s;
        (*pNewNode).pNext = pTop;
        (*pTop).pPrev = pNewNode;
        (*pNewNode).pPrev =NULL;
        pTop=pNewNode;
    }
}

void StringList::addToBottom(string s) // add to bottom
{
    if(isEmpty())
    {
        StringListNode * pNewNode;
        pNewNode = new StringListNode;
        (*pNewNode).data = s;
        pTop=pNewNode;
        pBottom=pNewNode;
        (*pNewNode).pPrev = NULL;
        (*pNewNode).pNext = NULL;
    }
    else
    {
        StringListNode * pNewNode;
        pNewNode = new StringListNode;
        (*pNewNode).data = s;
        (*pBottom).pNext = pNewNode;
        (*pNewNode).pPrev =  pBottom;
        (*pNewNode).pNext =NULL;
        pBottom=pNewNode;
    }
}

string StringList::print()
{
    string result;
    StringListNode * pCurrent;
    pCurrent=pTop;
    while(pCurrent!=NULL)
    {
        result+=(*pCurrent).data+"\n";
        pCurrent=(*pCurrent).pNext;
    }
    return result;
}

void StringList::clear()
{
    pTop = NULL;
    pBottom = NULL;
}

void StringList::remove(string s)
{
    StringListNode *curr = this->find(s);
    if (curr->pPrev != 0)
        curr->pPrev->pNext = curr->pNext;
    if (curr->pNext != 0)
        curr->pNext->pPrev = curr->pPrev;
    if (pTop == curr)
        pTop = curr->pNext;
    if (pBottom == curr)
        pBottom = curr->pPrev;
}

My program crashes when i try to read from my file. Read my comments within my constructor — WestonBuckeye, Jul 16 '13 at 20:32

Thomas Matthews · Accepted Answer · 2013-07-16T20:54:56.630

1

You have an error in your constructor, specifically a memory leak:

for each string s  // read from file but how do i write this?
{
    cur = new StringListNode(s);  // call the constructor of StringListNode
    cur = cur->pNext;
}

You create a node, but you immediately replace the pointer to the node with the node's initial next pointer.

As far as reading values from a file, I suggest you search Stack Overflow for "[c++] read from file". There have been too many of these lately.

Edit 1: Constructor correction

while (read data succeeds)
{
   add(data); // why not use your own function?
}

To make the append more efficient, you may want to maintain a pointer to the last node.

void append(data)
{
   p = new node(data);
   last->next = p;
   last = p;
}

edited Jul 16 '13 at 20:54

answered Jul 16 '13 at 20:40

Thomas Matthews

56,849
17
98
154

Ok. Starting from scratch on the constructor. Could you show me how to write this? I know how to read from a file in any ordinary case i just want to know how to do it and store those strings in a linked list. Thank you! – WestonBuckeye Jul 16 '13 at 20:48
And just for learning purposes, could you explain to me what a memory leak is? In this situation I mean. – WestonBuckeye Jul 16 '13 at 20:51
See my latest edit. I don't understand, you know how to add items to the list and you know how to read from a file, so what's the issue? – Thomas Matthews Jul 16 '13 at 20:59
A memory leak is when you allocate dynamic memory and lose the pointer to (or address of) the object that you allocated. In your post, `cur` points to the object, then you replace the pointer with `pNext` field of the new node, which loses the address of the allocated item. – Thomas Matthews Jul 16 '13 at 21:01
Wow, way to make me look like an idiot! haha just kidding! I used my add function. Thanks for your help my friend and for the explanation! I hope to understand this as much as you do one day! @Thomas Matthews – WestonBuckeye Jul 16 '13 at 21:18

GiBiT · Answer 2 · 2013-07-17T03:35:46.773

I did notice in your remove function you are forgetting an if statement. What would happen if something you enter is not in the list?

I believe you need something similar to this

if (curr == 0)
{
    return;
}

Also try for the in/output for your files

ifstream in;
in.open("Read.txt");

StringListNode * pCurrent;
pCurrent = new StringListNode;
pCurrent = pTop;    

while(pCurrent && (in >> pCurrent->data)) 
{
   pCurrent = pCurrent->pNext;
}    
in.close();


StringList::~StringList()
{
ofstream out;
out.open("Read.txt");

StringListNode * pCurrent;
pCurrent = new StringListNode;
pCurrent = pTop;
while(pCurrent != 0)  
{
    out << pCurrent->data << endl;
    pCurrent = pCurrent->pNext;
}
out.close();
}

That output should work. I just verified that. I think that that should fix your crashing issue with the input.

Constructor function reads from file and stores in linked list

2 Answers2