CODEWITHSUNDEEP
c++listfunctionparameterslinked-list
hp456
hp456

Reputation: 13

Looping through linked list C++ and returning the highest value of user input

The user inputs values of int age, and the function is supposed to accept parameters from a pointer to the head of the list, and go through the linked list nodes and return the highest value of int age.

Heres my function; I keep getting random numbers as the output:

int findLargest (StudentCard *p) {
    int current = p->age;
    int next;

    StudentCard *temp = p;

    while(temp != NULL){
        if (p->age == NULL) {
            //The value at this node is obviously larger than a non-existent value
            return current;
        } else {
            //Recur to find the highest value from the rest of the LinkedList
            next = findLargest(p->next);
        }
        //Return the highest value between this node and the end of the list
        if (current > next) {
            return current;
        } else {
            return next;
        }
        temp=temp->next;
    }
}

Upvotes: 0

Views: 2526

Answers (2)

Farhad Reza
Farhad Reza

Reputation: 446

First of all, you should return -1 at the end of your function to inform that nothing was found.

Then secondly, you are sending the p->next as the parameter of findLargest function without checking NULL in following code:

//Recur to find the highest value from the rest of the LinkedList
next = findLargest(p->next);

And also, when you are using recursion function call, you don't need the while(temp != NULL) loop at all. You could replace the while with a if and remove the temp = temp->next; statement. Either recursion or iteration is enough for solving your problem.

Your findLargest function should be as following code:

int findLargest (StudentCard *p) 
{
    if (p != NULL)
    {
        int current = p->age;
        int next = findLargest(p->next);

        if (current > next) {
            return current;
        } 
        else {
            return next;
        }
    }
    return -1;
}

For getting the oldest student node pointer, use following definition:

StudentCard * findLargestNode(StudentCard *p) 
{
    if (p != NULL)
    {
        int curAge = p->age;

        StudentCard *next = findLargestNode(p->next);

        if (next && (next->age > curAge)) {
            return next;
        } 
        else {
            return p;
        }
    }
    return NULL;
}

And for printing the oldest student ID, use the function as following

{
    StudentCard *pOld = findLargestNode(listHead); // `listHead` is HEAD node of your link-list
    if ( pOld )
        cout << pOld->id << endl;
}

Upvotes: 0

molbdnilo
molbdnilo

Reputation: 66371

You're mixing iteration with recursion, which is usually not a good idea.

(Your compiler should have warned about possibly not returning a value from the function. )

You're also possibly dereferencing a null pointer here:

int current = p->age;

and comparing the wrong thing here:

if (p->age == NULL)

(The fact that the program doesn't crash makes me suspect that you have an object with zero age somewhere, causing you to return that zero instead of recursing.)

If you read the loop carefully, you'll notice that it always returns a value on the first iteration, so temp is never advanced, and the while could be replaced with if.

You should rewrite the function to be either iterative or recursive.

An iterative solution would look like this:

int findLargest (StudentCard *p)
{
    int current = std::numeric_limits<int>::min();
    while (p != NULL){
        if (p->age > current) {
            current = p->age;
        }
        p = p->next;
    }
    return current;
}

and a recursive solution would look like this:

int findLargest (StudentCard *p)
{
    if (p == NULL) {
        return std::numeric_limits<int>::min();
    }
    return std::max(p->age, findLargest(p->next));
}

Upvotes: 2

Related Questions