1

This is a homework assignment!

I am tasked with creating two doubly linked lists, one for even numbers and one for odd numbers. The lists will be populated by reading in from a file of integers. I must search the correct list (either even or odd, depending on the number from the file) to see if the number read from the file is already in the list. If it is, I have to ignore the number and move on. We are not allowed to use OOP, but are working on the concept of ADT's. Please do not include any OOP in your answer as I am not allowed to use it, and I have no clue how to read it yet.

When I run my code it compiles without errors, but it returns a -1. After doing some debugging, I find that it is reading in and inserting the first number for each list, but then getting hung up during the insertion of the second number. However, I can't seem to figure out what is causing it to exit with -1. If anyone could point me in the right direction as to where the problem is I would greatly appreciate it.

Oh, also, the program takes the file names of the file you want to read the integers from and another file that will be used later in the program (I have not gotten to that part yet) as command line arguments.

#include <iostream>
#include <fstream>
#include <cstddef>
#include <iomanip>

using namespace std;

struct doublyLinkList{
    int numberRead;
    doublyLinkList* prev;
    doublyLinkList* next;
    };

struct topAndCounter{
    int counter;
    doublyLinkList* head;
    };

void initializeList(topAndCounter& aLinkList);
bool searchOrderedList(topAndCounter aLinkList, int tempIntHold);
void orderedListInsertion(topAndCounter& aLinkList, bool& success, int tempIntHold);
bool isEmptyList(topAndCounter& aLinkList);

int main(int argC, char* argV[])     // allows usage of command line arguments
{

    ifstream intsForList;
    ifstream intsToSearchFor;
    topAndCounter evenList;        // even list struct
    topAndCounter oddList;         // odd list struct
    int tempIntHold;               // holds integer read in from file
    string storeIntsFile;          // holds name of file
    string intsToFindFile;         // holds name of file
    bool success = true;           // whether memory was successfully allocated


    cout << endl << endl;
    cout << "Program to read in and store a list of unique integers from one ";
    cout << "file and match numbers to it from a second file." << endl << endl;


    switch(argC) // takes the argument count
    {
    case 1:
        cout << "You didn't enter any filenames on the command line." << endl;
        cout << "Please enter them now" << endl;
        cout << "Filename for list of integers to store: ";
        cin >> storeIntsFile;
        cout << "Filename for integers to search list for: ";
        cin >> intsToFindFile;
        break;
    case 2:
        cout << "You didn't enter the file name of integers to search the list";
        cout << " for" << endl;
        cout << "Please enter it now: ";
        cin >> intsToFindFile;
        storeIntsFile = argV[1];
        break;
    case 3:
        storeIntsFile = argV[1];
        intsToFindFile = argV[2];
        break;
    }

    intsForList.open(storeIntsFile.c_str());        // trys to open first file
    intsToSearchFor.open(intsToFindFile.c_str());   // tries to open second file

    // if files are not opened successful        
    if ((!intsForList) || (!intsToSearchFor))
    {
        while(!intsForList)  // while the first file is not opened successfully
        {
            cout << "File for integers to populate list not found. " << endl;
            cout << "Please enter another filename: ";
            cin >> storeIntsFile;

            intsForList.open(storeIntsFile);  // try to open file
        } // end while
        while(!intsToSearchFor)  // while the second file is not opened successfully
        {
            cout << "File containing integers to search list for not found.";
            cout << endl;
            cout << "Please enter another filename: ";
            cin >> intsToFindFile;

            intsToSearchFor.open(intsToFindFile);  // try to open file
         } // end while
    } // end if

    initializeList(evenList);       // function call to initialize list to NULL
    initializeList(oddList);        // function call to initialize list to NULL

    intsForList >> tempIntHold;     // read in the first integer from the file

    while((intsForList) && (success))   // while there is still integers in the file and memory 
                                        // allocated successfully
    {

        if (tempIntHold % 2 == 0)      // if the number read from the file is even
        {

            orderedListInsertion(evenList, success, tempIntHold);  // call the even sort funct
        } // end if
        else                         // if the number is odd
        {

            orderedListInsertion(oddList, success, tempIntHold);   // odd num sort funct
        } // end else

        intsForList >> tempIntHold; // try to read next integer in file
    } // end while


    return 0;
}

void initializeList(topAndCounter& aLinkList)
{
    aLinkList.counter = 0;
    aLinkList.head = NULL;                     // initialize head pointer to null

    return;
}

// function to search the list to see if the number read in from file is already in list
bool searchOrderedList(topAndCounter aLinkList, int tempIntHold)
{
    bool found = false;
    doublyLinkList* searchNode;                      // pointer for searching list

    searchNode = aLinkList.head;                     // set pointer to top of list

    while ((searchNode != NULL) && (!found))       // there are nodes in list and the number is 
                                                   // not found
    { 
        if (searchNode -> numberRead >= tempIntHold) 
        {
            found = true;
        }
        else
        {
            searchNode = searchNode -> next;  // move pointer to next node
        }
    }

    if (found) 
    {
         found = (searchNode -> numberRead == tempIntHold);
    }

    return found;

} // end function

void orderedListInsertion(topAndCounter& aLinkList, bool& success, int tempIntHold)
{
    doublyLinkList* newNode;
    doublyLinkList* searchNode;
    bool intInUse;
    bool found;

    newNode = new (nothrow) doublyLinkList;

    if (newNode != NULL)
    {
        intInUse = searchOrderedList(aLinkList, tempIntHold);

        if (intInUse)
        {
            cout << "The current interger is already in the list.  It will now ";
            cout << "be  ignored" << endl;
        } // end if
        else
        {
            newNode -> numberRead = tempIntHold;

            if (isEmptyList(aLinkList))
            {
                newNode -> prev = NULL;
                newNode -> next = NULL;
                aLinkList.head = newNode;
            } // end if

            else
            {
                searchNode = aLinkList.head;
                found = false;

                while((searchNode != NULL) && (!found))
                {

                    if (searchNode -> numberRead > newNode -> numberRead)
                    {
                        found = true;
                    } // end if
                    else
                    {
                         searchNode = searchNode -> next;
                    } // end else
                } // end while

                if (searchNode == aLinkList.head)
                {
                    newNode -> next = searchNode;
                    newNode -> prev = NULL;
                    searchNode -> prev = newNode;
                    aLinkList.head = newNode;
                } // end if
                else
                {
                    if(searchNode == NULL)
                    {
                        newNode -> prev = searchNode -> prev;
                        newNode -> next = searchNode;
                    } // end if
                    else
                    {
                    newNode -> prev = searchNode -> prev;
                    newNode -> next = searchNode;
                    searchNode -> prev = newNode;
                    } // end else
                } // end else

                newNode -> next = searchNode;
                newNode -> prev = searchNode -> prev;
                searchNode -> prev = newNode;
            } // end else

            aLinkList.counter++;
            success = true;
        } // end else

    } // end if
    else
    {
        cout << "No more heap memory.  No more integers from the list will be";
        cout << " stored." << endl;

        success = false;
    } // end else

    return;

} // end function

bool isEmptyList(topAndCounter& aLinkList)
{

    if (aLinkList.head == NULL)
    {
        return true;
    }
    else
    {
        return false;
    }
} // end function
  • 1
    `However, I can't seem to figure out what is causing it to exit with -1` When you're given a linked list assignment, you should be drawing boxes and lines denoting the nodes/links, and then work out exactly what an insertion into the list will entail. This is to be done before you write one line of code. Did you do that? If you did, then where does the program deviate from your plan that you wrote on paper? – PaulMcKenzie Jul 11 '14 at 14:55
  • In addition, C++ is not Java. You're allocating memory all over the place, and not one call to `delete` is issued. Your function that attempts to insert into the list also has a big bug in terms of memory leak issues. You allocate memory for a node without knowing whether you'll ever insert it into the list. If the node isn't inserted in the list, you never deallocate the node that wasn't used. – PaulMcKenzie Jul 11 '14 at 15:00
  • I will definitely deal with the allocating a node without knowing whether I will insert it or not. Later in the program (the part I haven't gotten to) I will deallocate all memory from both lists before the program exits. I will also draw the boxes as I haven't done that yet, and see if that helps. – user3510842 Jul 11 '14 at 15:56
  • I can't say thank you enough. I went and drew out my boxes and figured out where my program was going wrong within about 30 minutes. From now on I am going to take this advice and use it every time I have to deal with linked lists. – user3510842 Jul 11 '14 at 17:11

2 Answers2

0

Ignoring the "it's homework so I can't do it right" aspects, it can come out something like this:

#include <set>
#include <vector>
#include <string>
#include <iostream>

int main() { 
    std::vector<std::set<int>> sets(2);
    std::vector<std::string> names {"Even numbers", "Odd numbers"};

    int num;

    while (std::cin >> num)
       sets[num % 2].insert(num);

    for (int i=0; i<sets.size(); i++) {
        std::cout << "\n" << names[i];
        for (int j : sets[i])
            std::cout << "\t" << j;
    }
}

And no, no OOP there. In fact, the primary designer of this part of the standard library (Alexander Stepanov) is pretty clear about the fact that he doesn't particularly like object orientation, and really has no time for it at all.

As far as the rest of the requirements go, sorry, but they're too boneheaded to contemplate, much less implement. Contrary to the apparent beliefs of all too many teachers, implementing a set as a linked list is an excruciatingly awful idea.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • I'm sorry, but what is a vector? I don't think we have gotten there yet. I am still pretty new to programming. – user3510842 Jul 11 '14 at 15:57
  • A `std::vector` is the C++ dynamic array class. Also, if you're new to programming, why the heck were you assigned in creating a linked list class in C++? That is not for a beginner programmer. I can say this -- I have yet seen a beginner C++ programmer get a linked list class correctly coded. Unfortunately, these beginners get graded an "A" on woefully buggy code that seems to work, thus giving them the false sense of accomplishment. – PaulMcKenzie Jul 11 '14 at 16:26
  • Im not totally new, but relatively speaking. This is only my second assignment working on linked lists. I am trying to not be one of the somewhat new programers that makes buggy programs that seem to work, hence asking here instead of just working around the issue. We have not been introduced to OOP yet as that is a separate class, we are only working on the concepts of creating ADT. I know it probably doesn't make a lot of sense to more seasoned programmers. – user3510842 Jul 11 '14 at 17:16
  • @PaulMcKenzie: I'll answer that with a question: are you just trying to finish your class, or do you really want to learn to program in C++? In the latter case, I'd recommend the [C++ Book List](http://stackoverflow.com/q/388242/179910), especially [Accelerated C++](http://www.amazon.com/Accelerated-C-Practical-Programming-Example/dp/020170353X). `std::vector` is one of the *first* things you should learn about in C++. – Jerry Coffin Jul 11 '14 at 18:22
  • @JerryCoffin - I guess you mean to direct that to user3510842. – PaulMcKenzie Jul 11 '14 at 19:44
0

I will not give you the whole answer here, but I do have a few comments that will hopefully help you resolve the issue on your own.

  1. I found an access violation (you dereference a NULL pointer around line 213):

    if(searchNode == NULL)
    {
        newNode -> prev = searchNode -> prev; // HERE, searchNode == NULL
        newNode -> next = searchNode;
    } // end if
    else
    {
       newNode -> prev = searchNode -> prev;
       newNode -> next = searchNode;
       searchNode -> prev = newNode;
    } // end else
    
  2. It also strikes me that you have a lot of code that should be factored out into smaller functions.

    My tip to you is to rewrite your code into much smaller functions each having a very specific task.

    For example:

    // Inserts an element, and returns a pointer to the newly inserted element.
    doublyLinkList * insert_at_end( topAndCounter &head, int val );
    doublyLinkList * insert_before( topAndCounter &head, doublyLinkList *existing, int val );
    

    To put it this way, if you had the two functions I mentioned above, all of your code in orderedListInsertion could simplify to:

    void orderedListInsertion(topAndCounter& aLinkList, bool& success, int tempIntHold) {
        success = false;
        doublyLinkList *current = aLinkList->head;
    
        while (current != NULL) {
           if (current->numberRead == tempIntHold ) // Duplicate
               return;
           else if ( tempIntHold < current->numberRead ) {
                    insert_before(aLinkList, current, tempIntHold);
                    success = true;
                    return;
           }
        }
        insert_at_end(aLinkList, tempIntHold);
        success = true;
    }
    

    Better still it is much easier to test the small functions beforehand so you know for certain which parts of your program works.

Stian Svedenborg
  • 1,797
  • 11
  • 27