-3

I'm writing a function to find all occurrences of a node in a linked list, the function will return the number of occurrences to the main function which will then display those occurrences. The program does compile but the it just freezes and nothing seems to happen when I enter the correct name to look for, if I enter the wrong name, which is not in the list, the findall function returns 0 and the rest of the program works fine. Please take a look.

main.cpp

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

#include "List.h"

void extra(list &);

/***********************************
 * Main
 * Test function - DO NOT CHANGE
 ***********************************/ 
void main()
{
  list a;
  extra(a);
}

/***********************************
 * Extra Credit
 * Test function - DO NOT CHANGE
 ***********************************/ 

void extra(list &a)
{ int i,n;
  node_ptr map[4];
  string first,last;

// Find node

  cout << endl;
  cout << "Enter First and Last name: ";
  cin >> first >> last;
  n = a.findall(first,last,map,4);
 // Display forwards

  cout << endl;
  cout << "Find List\n--------------\n";
  for (i = 0; i < n; i++)
  {
      map[i]->put(cout);
  }
}

List.h

#include "Node.h"
#include <iostream>
#include <string>
using namespace std;

class list 
{  public:
        list();                                             // Empty constructor
        ~list();                                            // Destructor

        int findall(string, string, node_ptr*, int);

        node *find(string, string);                         // Locate a note


    private:
        node *head;
};

Node.h

#include <iostream>
#include <string>
using namespace std;


class list;

class node
{ friend list;
  public:
    node();                           // Null constructor
    ~node();                          // Destructor 

    void put(ostream &out);           // Put

  private:
    string first,last;
    int age;
    node *next;
};

typedef node * node_ptr;

List.cpp

#include "List.h"
#include <iostream>
#include <string>
using namespace std;

/**
* Empty Constructor
*
*/

list::list()
{
    head = nullptr;
}

/**
* Destructor Constructor
*
*/

list::~list()
{  if (head == nullptr) return;

    node *p = head, *t;

    while (p) 
    {
        t = p;
        p = p->next;
        delete t;
    }

    head = nullptr;
}

/**
* Locate node
*
*/

node *list::find(string last, string first) 
{
    node *temp = head;
    while (temp)
    {
        if (temp->first == first && temp->last == last) return temp;
        temp = temp->next;
    }
    return nullptr;
}


/**
* Find all.
*
*/

int list::findall(string first, string last, node_ptr* map, int n)
{
    int ans;
    ans = 0;
    *map = find(first, last);
    while (*map != NULL)
    {
        ans++;
        *map = (*map)->next;
        *map = find(first, last);

    }
    return ans;
}

Node.cpp

#include "Node.h"
#include <iostream>
#include <string>
#include <iomanip>
using namespace std;

/**
* Empty Constructor
* 
*/

node::node()
{
    last  = "";
    first = "";
    age   = 0;
    next  = nullptr;
}


/**
* Destructor
*
*/

node::~node()
{ if (next != nullptr) next = nullptr; 
}

/**
* Put
*
*/

void node::put(ostream &out)
{ out << setw(14) << left << last << setw(14) << first << setw(10) <<   age << endl; 
}

I really appreciate your help. Thank you.

Minh Le
  • 33
  • 1
  • 6
  • 1
    Please [edit] your question to provide a [mcve]. – Baum mit Augen Oct 14 '16 at 00:52
  • What observations have you made when you stepped through your program, one line at a time, with your debugger? stackoverflow.com is not a web site where you get to dump massive amounts of code, and expect someone else to debug your own code for you. – Sam Varshavchik Oct 14 '16 at 00:54
  • 2
    The right tool to solve such problems is your debugger. You should step through your code line-by-line *before* asking on Stack Overflow. For more help, please read [How to debug small programs (by Eric Lippert)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). At a minimum, you should \[edit] your question to include a [Minimal, Complete, and Verifiable](http://stackoverflow.com/help/mcve) example that reproduces your problem, along with the observations you made in the debugger. – πάντα ῥεῖ Oct 14 '16 at 00:55
  • 1
    `while(!infile.eof())` will kick you in the teeth at some point. [Read more here.](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). TL:DR, the only way to know if you've hit the end of the file is to fail a read because you hit the end of the file. You need to read, then test, then use if read succeeded. – user4581301 Oct 14 '16 at 00:58

1 Answers1

3

findall() freezes because it gets stuck in an endless loop.

If no matching node is found, the first call to find() returns nullptr and findall() exits.

But if a matching node is found, a loop is entered, calling find() to search the entire list all over again from the beginning. That will find the same node as before. Then you call find() again, and again, and so on.

To solve this issue, if find() returns a matching node, you need to pass the next node in the following call to find() so it can start searching where the previous search left off. Repeat until you reach the end of the list.

class list 
{  public:
    ...

        int findall(string first, string last, node_ptr *map, int n);

        node_ptr find(string first, string last, node_ptr start = nullptr);               // Locate a note

    ...
};

node_ptr list::find(string last, string first, node_ptr start) 
{
    node_ptr temp = (start) ? start : head;
    while (temp)
    {
        if ((temp->first == first) && (temp->last == last)) break;
        temp = temp->next;
    }
    return temp;
}

int list::findall(string first, string last, node_ptr* map, int n)
{
    int ans = 0;
    node_ptr temp = nullptr;
    while (ans < n)
    {
        temp = find(first, last, temp);
        if (!temp) break;
        *map++ = temp;
        ++ans;
        temp = temp->next;
    }
    return ans;
}

Update: if you are not able to change the signature of find() then you will have to re-write findall() to duplicate what find() does:

class list 
{  public:
    ...

        int findall(string first, string last, node_ptr *map, int n);

        node_ptr find(string first, string last);               // Locate a node

    ...
};

node_ptr list::find(string last, string first) 
{
    node_ptr temp = head;
    while (temp)
    {
        if ((temp->first == first) && (temp->last == last)) break;
        temp = temp->next;
    }
    return temp;
}

int list::findall(string first, string last, node_ptr* map, int n)
{
    int ans = 0;
    node_ptr temp = head;
    while (ans < n)
    {
        while (temp)
        {
            if ((temp->first == first) && (temp->last == last)) break;
            temp = temp->next;
        }
        if (!temp) break;
        *map++ = temp;
        ++ans;
        temp = temp->next;
    }
    return ans;
}
Remy Lebeau
  • 454,445
  • 28
  • 366
  • 620
  • Thank you for your response. However, I'm trying to think of a way to do this without changing the find() because it can only take two arguments. I think I need to rewrite the findall function so that I don't have to use the find() since I can't change it. Thanks again for your help. – Minh Le Oct 14 '16 at 01:27
  • I updated my answer. – Remy Lebeau Oct 14 '16 at 02:24
  • Thanks again for your help. I understand most of your code for the findall(). However, I am not sure what this line of code does. – Minh Le Oct 14 '16 at 15:10
  • `*map++ = temp` As I understand it: increase the map ptr and assign the temp value to that ptr? Thank you. – Minh Le Oct 14 '16 at 15:11
  • The postfix `++` increment operator has a higher [precedence](http://en.cppreference.com/w/cpp/language/operator_precedence) than the `*` dereference operator, so the line is equivalent to `*(map++) = temp`. The postfix operator returns the previous value before the increment. The code is assigning `temp` to the current address that `map` is originally pointing at, and incrementing `map` to point at the next address. The line is equivalent to these statements: `node_ptr* temp_map = map; ++map; *temp_map = temp;` – Remy Lebeau Oct 14 '16 at 19:12