What else can i do to improve this c++ Code

Question

So I am trying to improve this code in c++. What this does it creates two classes: Student and Studentlist. Any suggestions on improving the linked list data structure here will be greatly appreciated.

#include <iostream>

using namespace std;

//declaring a class student 

class Student
{

public:
    char *RollNo;
    Student *next;
    //function student that includes arguments roll number and a pointer poniting to next node student

    Student(char *rollNo, Student *Next)
    {
        this->RollNo = rollNo;
        this->next = Next;
    }
    //fucntion to get roll number 
    char* getRollNo()
    {
        return RollNo;
    }
    //function to get the pointer to next node named student
    Student *getNext()
    {
        return next;
    }

    void setNext(Student *aNode)
    {
        this->next = aNode;
    }
};

//declareing a class StudentList

class StudentList
{
public:
    Student *head;
    // default constructor
    StudentList()
    {
        head = NULL;
    }
    void Add(char *aRollNo)
    {
        Student *newStudent = new Student(aRollNo, NULL);
        Student *temp = head;
        if (temp != NULL)
        {
            while (temp->getNext() != NULL)
            {
                temp = temp->getNext();
            }
            temp->setNext(newStudent);
        }
        else
        {
            head = newStudent;
        }
    }
    void display()
    {
        Student *temp = head;
        if (temp == NULL)
        {
            cout << "no student data in the Student List" << endl;
            return;
        }
        if (temp->getNext() == NULL)
        {
            cout << temp->getRollNo();
        }
        else
        {
            do
            {
                cout << temp->getRollNo() << " --next--> ";
                temp = temp->getNext();
            } while (temp != NULL);
            cout << " end --> null" << endl;
        }
    }
};

main()
{
    StudentList list;
    list.Add("My Roll Number is 411\n");
    list.display();
    cout << "--------------------------------\n";
    system("pause");
    return 0;
}

I think you're looking for [CodeReview](http://codereview.stackexchange.com/) — Barry, May 17 '15 at 23:18
As long as the code is working then it [CodeReview](http://codereview.stackexchange.com/) would be a good destination on how to improve the code. But currently the code does not compile. Get that fixed first, otherwise it will be closed as off topic. — Martin York, May 17 '15 at 23:26

score 0 · Answer 1 · edited May 23 '17 at 11:58

The declaration of main() is not complete.

 main()

See What is the proper declaration of main?

Also literal strings have a type of char const*. So your method call Add("XXX") has no matching point in the class. The closest you have is Add(char*) which does not match the const part.

Personally I would avoid using C-Strings in C++. You should look at using std::string to handle strings of characters it will avoid many problems.

Mohammad Al Baghdadi · Answer 2 · 2015-05-18T01:34:57.983

0

while you are always add at the last i recomended you to replace your Add algorithm code with this

Student* Add(char *aRollNo,Student* last)
{
    Student *newStudent = new Student(aRollNo, NULL);
    Student *temp = last;
    if (head == NULL){
        head=newStudent;
        temp=head;}
    else
        temp=temp->setNext(newStudent);
  return temp;     
}

edited May 18 '15 at 01:34

answered May 18 '15 at 01:27

Mohammad Al Baghdadi

377
2
21

if you are looking for speed this will be a fine algorithm for Add especially if you have a long list of Students then the while loop will take pretty long time of processor first you pass NULL then you pass the returning value – Mohammad Al Baghdadi May 18 '15 at 01:36

What else can i do to improve this c++ Code

2 Answers2