Have I completed this C++ pointers / lists assignment?

Write a class ListNode which has the following properties:

  • int value;
  • ListNode *next;

Provide the following functions:

  • ListNode(int v, ListNode *l)
  • int getValue();
  • ListNode* getNext();
  • void insert(int i);
  • bool listcontains(int j);

Write a program which asks the user to enter some integers and stores them as ListNodes, and then asks for a number which it should seek in the list.

Here is my code:

#include <iostream>
using namespace std;

class ListNode
{
    private:
        struct Node
        {
            int value;
            Node *next;
        } *lnFirst;

    public:
        ListNode();
        int Length();       
        void DisplayList();     
        void Insert( int num );
        bool Contains( int num );       
        int GetValue( int num );        
};

ListNode::ListNode()
{
    lnFirst = NULL;
}

int ListNode::Length()
{
    Node *lnTemp;
    int intCount = 0;
    for( lnTemp=lnFirst ; lnTemp != NULL ; lnTemp = lnTemp->next )
    {
        intCount++;
    }
    return intCount;
}

void ListNode::DisplayList()
{
    Node *lnTemp;
    for( lnTemp = lnFirst ; lnTemp != NULL ; lnTemp = lnTemp->next )
        cout<<endl<<lnTemp->value;
}

void ListNode::Insert(int num)
{
    Node *lnCurrent, *lnNew;

    if( lnFirst == NULL )
    {
        lnFirst = new Node;
        lnFirst->value = num;
        lnFirst->next = NULL;
    }
    else
    {
        lnCurrent = lnFirst;
        while( lnCurrent->next != NULL )
            lnCurrent = lnCurrent->next;

        lnNew = new Node;
        lnNew->value = num;
        lnNew->next = NULL;
        lnCurrent->next = lnNew;
    }
}

bool ListNode::Contains(int num)
{
    bool boolDoesContain = false;
    Node *lnTemp,*lnCurrent;
    lnCurrent = lnFirst;
    lnTemp = lnCurrent;
    while( lnCurrent!=NULL )
    {
        if( lnCurrent->value == num )
        {
            boolDoesContain = true;
            return boolDoesContain;
        }
        lnTemp = lnCurrent;
        lnCurrent = lnCurrent->next;
    }
    return boolDoesContain;
}

int ListNode::GetValue(int num)
{
    Node *lnTemp;
    int intCount = 1;
    for( lnTemp=lnFirst; lnTemp != NULL; lnTemp = lnTemp->next )
    {
        if (intCount == num)
        {
            return lnTemp->value;
        }
        intCount++;
    }    
}

int main()
{   
    cout << "Input integers below. Input the integer -1 to stop inputting.\n\n";

    ListNode lnList;
    int intNode = 1, intInput = 0;
    while (intInput != -1) {
        cout << "Please input integer number " << intNode << ": "; cin >> intInput;
        intNode++;
        if (intInput != -1) { lnList.Insert(intInput); }
    }   

    lnList.DisplayList();
    cout << "\n\n";

    int intListLength = lnList.Length();    
    cout << "Which value do you wish to recall? (Between 1 and " << intListLength << "): "; cin >> intNode;    
    if ( intNode >= 1 && intNode <= intListLength ) {
        cout << "Value at position " << intNode << " is " << lnList.GetValue(intNode) << ".";                
    } else {
        cout << "No such position in the list. Positions run from 1 to " << intListLength << ". You asked for " << intNode << ".";
    }

    cout << "\n\nCheck if the following value is in the list: "; cin >> intNode;
    bool IsThere = lnList.Contains(intNode);
    if (IsThere) {
        cout << intNode << " is in the list.";
    } else {
        cout << intNode << " is not in the list.";
    }

    cout << "\n\n";
    system("pause");
    return 0;  
}

Where can we improve this?

Answers


What unwind and ckarmann say. Here is a hint, i implement listcontains for you to give you the idea how the assignment could be meant:

class ListNode {
private:
    int value;
    ListNode * next;

public:
    bool listcontains(int v) { 
        // does this node contain the value?
        if(value == v) return true; 

        // was this the last node?
        if(next == 0) return false;

        // return whether nodes after us contain the value 
        return next->listcontains(v);
    }
};

So, you only have the head of the list, which links to the next node in turn. The tail will have next == 0;


I think you misunderstood the requested design. The ListNode class is supposed to be a node, not a list containing nodes.

I would advise you not to put several commands on a single ligne, like this:

cout << "Please input integer number " << intNode << ": "; cin >> intInput;

This is simply confusing.


On a more general style note, declare pointers closer to where you'll define them, and keep their scope as small as possible. While nothing can technically go wrong with your code, always doing this avoids bugs in much larger/ older codebases in my experience.

e.g. instead of

Node *lnTemp;
int intCount = 0;
for( lnTemp=lnFirst ; lnTemp != NULL ; lnTemp = lnTemp->next )
{
}

write

int intCount = 0;
for(Node* lnTemp=lnFirst ; lnTemp != NULL ; lnTemp = lnTemp->next )
{
}

or, similar, instead of

Node *lnTemp,*lnCurrent;
lnCurrent = lnFirst;
lnTemp = lnCurrent;

write

Node* lnCurrent = lnFirst;
Node* lnTemp = lnCurrent;

Need Your Help

metasploit: unrecognized service

debian metasploit

I have metasploit installed in opt/metasploit-framework (using debain wheezy) , but when i try to start the service using service metasploit start ,it shows an error (metasploit: unrecognized servi...

Remove elements of List<int> use View State in asp.net

asp.net viewstate

I am meeting a problem about View State. Here my code:

About UNIX Resources Network

Original, collect and organize Developers related documents, information and materials, contains jQuery, Html, CSS, MySQL, .NET, ASP.NET, SQL, objective-c, iPhone, Ruby on Rails, C, SQL Server, Ruby, Arrays, Regex, ASP.NET MVC, WPF, XML, Ajax, DataBase, and so on.