Can anyone help me see why my pointer function is not working properly?

[ovaltine99]

My professor tells me that my "push_back" is not working and I get a memory error after entering a 2nd course because X's "next" pointer is uninitialized. I'm a bit confused on how I'm supposed to go about initializing the x's next pointer?

#include <iostream>
using std::cin;
using std::cout;
using std::endl;
using std::ios;

#include <iomanip>
using std::setw;

#include <fstream>
using std::ofstream;
using std::ifstream;

#include <cstring>

#include <cstdlib>

class course
{
public:
  char classDes[11]; //course designation ex. COMSC-165, accomodate 10 characters
  char term[7]; //term ex. FA2010
  int units; //units
  char grade; //grade
  course* next; //prepare your class to be used in a linked list by adding a next pointer
};

//prototypes
void dispCourse(course*);
int courseCmp(const course*, const course*);
course* restoreList(course*);
course* sortByUnit(course*);
course* sortByGrade(course*);
course* sortByDescription(course*);
course* sortByTerm(course*);
course* deleteList(course*);
void saveList(course*);
course* push_back(course*, course*);

int main()
{
	//variables
	char classPrompt; 
	char load; 
	char buf[100];
	
	course* head = 0; //initially empty list
	
	//ask if you want to load from disk
	cout << "Load previous entered classes? (Y/N): ";
	cin >> load;

	//yes, load and print 
	if (load == 'Y' || load == 'y')
	{
		head = restoreList(head);
		dispCourse(head);
	} 

	while(true)
		{
			//prompt user to enter class
			cout << "Would you like to add a class to the course list? (Y/N): ";
			cin >> classPrompt;
			cin.ignore(1000, 10);

			if (classPrompt == 'N' || classPrompt == 'n')
			{
				cout << endl << "Thank you for entering your courses" << endl;
				break;
			} 

			else
			{
				
				course* temp = new course;
				cout << endl;

				dispCourse(head);
				cout << "Please enter the course name: "; 
				cin.getline(temp->classDes, 11);
				
				cout << "Please enter the course term: "; 
				cin.getline(temp->term, 7);
				
				cout << "Please enter # of units: ";
				cin >> temp->units;
				cin.ignore(1000, 10);
 
				cout << "Please enter your grade: "; 
				cin >> temp->grade; 
				cin.ignore(1000, 10); 
				head = push_back(head, temp);
			} 
		}

	saveList(head);
	
	cout << endl;
	cout << "BY UNIT-" << endl;
	head = sortByUnit(head);
	dispCourse(head);
			
	cout << endl;
	cout << "PRESS ENTER TO CONTINUE" << endl;
	cin.get();

	cout << "BY GRADE-" << endl;
	head = sortByGrade(head);
	dispCourse(head);

	cout << endl;
	cout << "PRESS ENTER TO CONTINUE" << endl;
	cin.get();

	cout << "BY DESCRIPTION-" << endl;
 	head = sortByDescription(head);
	dispCourse(head);

	cout << endl;
	cout << "PRESS ENTER TO CONTINUE" << endl;
	cin.get();

	cout << "BY TERM-" << endl;
	head = sortByTerm(head);
	dispCourse(head);

  head = deleteList(head);
} 
 
void dispCourse(course* p)
{
	if (p)
	{
		
		cout << setw(10) << "Course" << setw(10) << "Term" << setw(10) << "Units" << setw(10) <<  "Grade" << endl;
		cout << setw(10) << "-----" << setw(10) << "-----" << setw(10) << "-----" << setw(10) << "-----" << endl;

		for(course* i = p; i;i = i->next)
		{
			cout.setf(ios::right, ios::adjustfield);
			cout << setw(10) << i->classDes;
			cout << setw(10) << i->term;
			cout << setw(10) << i->units;	
			cout << setw(10) << i->grade << endl;        
		} 
	}
} 

int courseCmp(const course* a, const course* b)
{
	//validate the length of the strings
	if ((strlen(a->term) != 6) || (strlen(b->term) != 6)) //...in cstring
		return strcmp(a->classDes, b->classDes);
	
	//handle ties here
	if (stricmp(a->term, b->term) == 0)
		return stricmp(a->classDes, b->classDes);

	//compare the year
	int yearA = atoi(a->term + 2); //atoi is found
	int yearB = atoi(b->term + 2); //in cstdlib
	if (yearA < yearB)
		return -1; //termA comes first

	if (yearA > yearB)
		return 1; //termB comes first

	//compare semester in case of same year
	if (strnicmp(a->term, "SP", 2) == 0)
		return -1; //termA comes first

	if (strnicmp(a->term, "SU", 2) == 0)
		return strnicmp(b->term, "SP", 2) ? -1 : 1;
	return 1;
} 

course* deleteList(course* dList)
{
	while(dList)
	{
		course* x = dList->next;
		delete dList;
		dList = x;
	}
	return dList;
} 

course* push_back(course* head, course* x)
{
	course* t = head;
	if (t == 0)
	{
		x->next = 0;
		return x;
	}

	else
	{
		course* a = t;
		while(a->next)
		{
			a = a->next;
		} 
		a->next = x;
		return t;
	} 
} 

course* restoreList(course* head) //add persistence (step 2)
{
	ifstream fin;
	fin.open("courses.dat", ios::binary|ios::in);

	if (!fin)
		return head;

	//read the number of objects from the disk file
	int nRecs;
	fin.read((char*)&nRecs, sizeof(int));

	//read the objects from the disk file
	course* newHead = head;
	for (int i = 0; i < nRecs; i++)
	{
		course* x = new course;
		fin.read((char*)x, sizeof(course));
		x->next = 0;
		newHead = push_back(newHead, x);
	} 
	fin.close();
	return newHead;
} 

void saveList(course* head)
{
	int counter = 0;
	ofstream fout;
	fout.open("courses.dat", ios::binary|ios::out|ios::trunc);
	course* x = head;

	for(counter, x; x; ++counter, x = x->next);
	x = head;
	fout.write((char*)&counter, sizeof(int));
	for (x; x; x = x->next)
	{
		fout.write((char*)x, sizeof(course));
	}
	fout.close();
} 

course* sortByUnit(course* head)
{
	course* newHead = 0;
	course* oldHead = head;

	while(oldHead)
	{
		course* a = oldHead;
		oldHead = oldHead->next;

		course* x, *prev;

		for (x = newHead, prev=0; x; prev = x, x = x->next)
		{
			if (a->units < x->units)
				break;
		}
		 
		a->next = x;

		if (prev)
			prev->next = a;

		else
			newHead = a;
	}
	return newHead;
} 

course* sortByGrade(course* head)
{
	course* newHead = 0;
	course* oldHead = head;

	while (oldHead)
	{
		course* a = oldHead;
		oldHead = oldHead->next;
		course* x, *prev;

		for (x = newHead, prev=0; x; prev = x, x = x->next)
		{
			if (a->grade < x->grade)
				break;
		} 

		a->next = x;

		if (prev)
			prev->next = a;

		else
			newHead = a;
	} 
	return newHead;
} 

course* sortByDescription(course* head)
{
	course* newHead = 0;
	course* oldHead = head;

	while (oldHead)
	{
		course* a = oldHead;
		oldHead = oldHead->next;
		course* x, *prev;

		for (x = newHead, prev = 0; x; prev = x, x = x->next)
		{
			if (strcmp(a->classDes, x->classDes) < 0)
				break;
		}

		a->next = x;
		if (prev)
			prev->next = a;

		else
			newHead = a;
	} 
	return newHead;
}

course* sortByTerm(course* head)
{
	course* newHead = 0;
	course* oldHead = head;

	while (oldHead)
	{
		course* a = oldHead;
		oldHead = oldHead->next;
		course* x, *prev;

		for (x = newHead, prev = 0; x; prev = x, x = x->next)
		{
			if (courseCmp(a, x) < 0)
				break;
		}

		a->next = x;
		if (prev)
			prev->next = a;

		else
			newHead = a;
          } 
	return newHead;
} 

[Smac89]

line 205 should be:

a->next = new course;
a->next = x;

[D80DSL3A]

Add x->next = 0; following line 205.
@Smac. His line 205 is fine. Note that the courses are allocated outside of the push_back() function and then passed in as x. This is unusual, but it could work.

[keskiverto]

x->next = 0; is conceptually awful in push_back(). Suppose you would want to concatenate two lists. push_back does it -- almost. It just happens to leak all but the first item of list 'x'.

x->next = 0; is potential error in push_back(). 't' can be 0, but what if 'x' is 0?


One should initialize the 'next' to 0 in the constructor of 'course'. Better yet, initialize all data members of 'course' in its constructor.

[D80DSL3A]

x->next = 0; is necessary. This marks the end of the list, which is what OPs professor told him is missing.
I agree that it would be better to initialize all members in a constructor, but OP hasn't written one. It would also be better to allocate the Node (known here as a 'course') within the push_back() function, rather than outside of it then passing it in. This would insure that x != 0.
If x=0 is passed to the function then yes, x->next would give segfault.

[coder777]

on line 194 you initialze x->next = 0;.
This line is missing in the else branch. Insert x->next = 0; between line 205 and 206

[ovaltine99]

Thanks for the help guys! Appreciate it :)

[ovaltine99]

Hey guys thanks for the help but my professor pointed out another problem I've got. He says "You are supposed to sort by case INDEPENDENT grade, and you sort by case DEPENDENT grade instead . Also, you do not print the whole list before asking the user if they want to add a course"

Could anyone help me out with sorting by case INDEPENDENT grade? Also, the last pat about printing the entire list before the user is prompted, I did this before my simply printing the function before like dispCourse(course* p) but of course this isn't going to work in my case. I'm having trouble printing out just the list now.

Thanks again!

[Smac89]

What trouble are you experiencing with the function?

[ovaltine99]

I'm not having trouble with the function but I'm having trouble outputting the list out before the user enters anything such as before they are prompted. For example I want to output the function

 
void dispCourse(course* p)
{
	if (p)
	{
		
		cout << setw(10) << "Course" << setw(10) << "Term" << setw(10) << "Units" << setw(10) <<  "Grade" << endl;
		cout << setw(10) << "-----" << setw(10) << "-----" << setw(10) << "-----" << setw(10) << "-----" << endl;

		for(course* i = p; i;i = i->next)
		{
			cout.setf(ios::right, ios::adjustfield);
			cout << setw(10) << i->classDes;
			cout << setw(10) << i->term;
			cout << setw(10) << i->units;	
			cout << setw(10) << i->grade << endl;        
		} 
	}
} 

I want to output it right here in the while statement before the user is even prompted I want them to see the function.

while(true)
		{
			//prompt user to enter class
			cout << "Would you like to add a class to the course list? (Y/N): ";
			cin >> classPrompt;
			cin.ignore(1000, 10);

			if (classPrompt == 'N' || classPrompt == 'n')
			{
				cout << endl << "Thank you for entering your courses" << endl;
				break;
			} 

[ovaltine99]

Any tips would be greatly appreciated I still am unable to get the displaying the

void dispCourse(course* p)

to work yet. Am I missing something very obvious that is needed in order to display this?