I've gotten feedback from my instructor to modify my code because it contains poor design techniques. I need to add a " return " after each function without it going back to " main " and also to avoid the use of global headers.

I've tried to modify it several ways, but I cannot seem to do it. Any help would be greatly appreciated as I am still learning the c++ language.

# include<iostream.h>
# include<stdlib.h>
# include<conio.h>
# include<string.h>


struct ListEntry
{
    char name[10];
    int idnumber;
    struct ListEntry *next;
}   
    start,*node,*prev;

void add();
void viewdb();

void main()

{
    int choice;

    system("cls");
          cout<<"Choose option from the menu below:\n\n";
          cout<<"[1] Insert an ID Number and Name\n";
          cout<<"[2] View the database\n";
          cout<<"[3] Exit\n";
          cout<<"\t\tEnter your choice: ";
          cin>>choice;

          switch(choice)
         {
          case 1:
               add();
               break;
          case 2:
               viewdb();
               break;
          case 3:
               exit(1);


          }
}

void add()
{
          char choice;
          start.next=NULL; 
          node=&start; 
              do
             {

              system("cls");
              node->next=(struct ListEntry*)malloc(sizeof(struct ListEntry));
              node=node->next;

              cout<<"Enter an ID Number and Name:\t";
              cin>>node->idnumber>>node->name;

              cout<<"Record saved. Input another? (Y,N): ";
              cin>>choice;

              node->next=NULL;
              }

              while(choice=='Y'||choice=='y');
                   main();
              } 

void viewdb()
{

              node=start.next;
              system("cls");
              while(node)
              {
                     cout<<node->idnumber<<"\t"<<node->name<<endl;
                     node=node->next;
              }

              cout<<endl;
              getch();
              main();
}

<< moderator edit: added code tags: [code][/code] >>

Dani AI

Generated

Two separate bugs drive most of the trouble in the thread. Calling main() from add() and viewdb() to "return" control is incorrect; those functions should simply return and the menu loop should live in main. already pointed this out. The second problem is that the global header/sentinel (the start node) is being reinitialized when inserting records, which discards previously added nodes. The unused global variable prev should be removed or given a purpose. Removing globals and owning the head pointer in a single place prevents these mistakes.

A straightforward fix is to stop using a global head and either:

  • keep a head pointer in main and pass it by reference to add/view functions (Node *&), or
  • wrap the list in a small class that owns the head pointer and frees nodes in its destructor (RAII). The class approach prevents leaks and centralizes list logic.

Example of the class approach (minimal, modern C++ style):

#include <iostream>
#include <string>

struct Node {
    int id;
    std::string name;
    Node* next;
    Node(int i, const std::string& n): id(i), name(n), next(nullptr) {}
};

class SimpleList {
    Node* head;
public:
    SimpleList(): head(nullptr) {}
    ~SimpleList() { while (head) { Node* t = head; head = head->next; delete t; } }
    void add(int id, const std::string& name) {
        Node* n = new Node(id, name);
        if (!head) head = n;
        else { Node* p = head; while (p->next) p = p->next; p->next = n; }
    }
    void view() const { for (Node* p = head; p; p = p->next) std::cout << p->id << '\t' << p->name << '\n'; }
};

Practical notes: prefer int main() and loop the menu there; use std::string (not fixed char arrays) and std::getline for names with spaces; avoid malloc/free in C++ (use new/delete or smart pointers); separate UI (clearing screen, getch) from data logic. Applying these changes removes the global header node, preserves previously inserted data across menu operations, and makes memory management predictable.

Recommended Answers

All 5 Replies

yaan,

Inside your function definitions, you need to replace "main()" with "return".

You are calling the main function instead of actually returning to it.

You should also add some white space to your code so it is more readable. See below.

Take care,
Bruce

# include<iostream.h>
# include<stdlib.h>
# include<conio.h>
# include<string.h>


struct ListEntry
{
	char	name[10];
	int		idnumber;
	struct	ListEntry *next;
} 
start,*node,*prev;

void add();
void viewdb();

void main()

{
	int choice;

	system("cls");
	cout << "Choose option from the menu below:\n\n";
	cout << "[1] Insert an ID Number and Name\n";
	cout << "[2] View the database\n";
	cout << "[3] Exit\n";
	cout << "\t\tEnter your choice: ";
	cin  >> choice;

	switch(choice)
	{
		case 1:
			add();
			break;
		case 2:
			viewdb();
			break;
		case 3:
			exit(1);


	}
}



void add()
{
	char choice;

	start.next=NULL; 

	node=&start; 

	do

	{



		system("cls");

		node->next=(struct ListEntry*)malloc(sizeof(struct ListEntry));

		node=node->next;


		cout<<"Enter an ID Number and Name:\t";

		cin>>node->idnumber>>node->name;


		cout<<"Record saved. Input another? (Y,N): ";

		cin>>choice;


		node->next=NULL;

	}


	while(choice=='Y'||choice=='y');

	return;

} 






void viewdb()
{


	node=start.next;

	system("cls");

	while(node)

	{

		cout<<node->idnumber<<"\t"<<node->name<<endl;

		node=node->next;

	}


	cout<<endl;

	getch();

	return;
}

<< moderator edit: added code tags: [code][/code] >>

Yaan,

Eliminating the calls to main is correct.

Your logic must be updated to allow your main menu to be shown until exit.

I have provided an update to your code below.

The update also seperates the requests for id and name.

Take care,
Bruce

# include<iostream.h>
# include<stdlib.h>
# include<conio.h>
# include<string.h>


struct ListEntry
{
	char	name[10];
	int		idnumber;
	struct	ListEntry *next;
} 
start,*node,*prev;

void add();
void viewdb();

void main()

{
	int choice;


	do
	{

	system("cls");
	cout << "Choose option from the menu below:\n\n";
	cout << "[1] Insert an ID Number and Name\n";
	cout << "[2] View the database\n";
	cout << "[3] Exit\n";
	cout << "\t\tEnter your choice: ";
	cin  >> choice;

	switch(choice)
	{
		case 1:
			add();
			break;
		case 2:
			viewdb();
			break;
		//case 3:
		//	exit(1);
	}

	} while (choice != 3);


}



void add()
{
	char choice;

	start.next = NULL; 

	node = &start; 

	do

	{
		system("cls");

		node->next = (struct ListEntry*) malloc(sizeof(struct ListEntry) );
		node = node->next;

		cout << "Enter an ID Number:\t";
		cin  >> node->idnumber;

		cout << "Enter a Name:\t";
		cin  >> node->name;


		cout << "Record saved. Input another? (Y, N): ";
		cin >> choice;


		node->next = NULL;

	} while (choice == 'Y' || choice == 'y');

	return;

} 






void viewdb()
{


	node = start.next;

	system("cls");

	while (node)

	{

		cout << node->idnumber << "\t" << node->name << endl;

		node = node->next;

	}


	cout << endl;

	getch();

	return;
}

<< moderator edit: added code tags: [code][/code] >>

I see how adding more space makes the code easier to read. I tried your modifications and they worked. I was adding a " return 0; " instead of just a " return; "

Would you happen to know how to remove the global header node?

Thanks Bruce.

Yaan,

There is another bug in your code.

Each time you start inserting records you are setting start.next = NULL. This destroys your pointer to any previously entered data.

Try inserting data, go back to main menu, insert more data, go back to main menu and view the db. The first set of data is gone.

Take care,
Bruce

You aren't even using the variable "prev".

Do you really need the "start" variable?

Bruce

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.