View Poll Results: What can I do different about my code to make it better
- Voters
- 0. You may not vote on this poll
-
December 10th, 2010, 07:50 PM
#1
C++ Linked List Issues
I am having trouble with my linked list. My teacher wants us to add the total number of nodes, the total from the user input (example: if the user enters 5, 5, ,5 she wants the results displayed as 15. As far as counting the nodes I have no idea how to implement it inside my code or the math to figure it out. If someone can help me I would greatly appreciate it! Here is my code as of now:
/*
1. Write a program that allows the user to enter a series of positive
floating-point numbers.
2. Store the numbers in a linked list.
3. Allow the user to enter numbers until a negative number is entered. (terminate when a negative # is entered).
4. Next, transverse the list to total the numbers entered, number of nodes, and average to the screen
*/
#include <iostream>
#include <iomanip>
using namespace std;
struct node
{
float value;
node *next;
};
node *head_ptr = NULL;
node *current_ptr = NULL;
int get_user_data ();
void add_node (float number);
void move_current_to_end();
void display_list();
void delete_list();
int main()
{
float numbers;
while(get_user_data());
display_list(); // display the users input and total
return 0;
}
int get_user_data ()
{
float input = 1;
if (input != 0)
{ // Display details for what temp points to
cout << "Please insert a number: " << endl;
cin >> input;
add_node(input);
cout << endl; // Blank line
}
else
{
input = 0;
}
return (input);
}
void add_node (float number)
{
node *temp, *temp2; // declare local pointer for new node
temp = new node; // allocate memory for a new node and
// initialize pointer to point to it
temp->value = number;
temp->next = NULL; //new node ptr to null (end of list)
if(head_ptr == NULL)
head_ptr = temp;
else
{
temp2 = head_ptr; //start at head of list
while (temp2->next != NULL)
{
temp2 = temp2->next; //advance to next node
}
temp2->next = temp; //found the end, append temp (the new node)
}
}
void move_current_to_end()
{
current_ptr = head_ptr; // move current_ptr to head of the list
while (current_ptr->next!= NULL)
{
// transverse list until NULL is reached
current_ptr = current_ptr->next;
}
}
void display_list()
{
current_ptr = head_ptr; // move current_ptr to head of list
cout << "Numbers" << endl;
do
{
cout.setf(ios::left);
cout << setw(5) << current_ptr->value << " - " << endl;
current_ptr = current_ptr->next; // point current_ptr to next node
} while (current_ptr != NULL); // loop until end of list
cout<<endl;
}
void delete_list()
{
node *temp_ptr; // pointer used for temporary storage
current_ptr = head_ptr; // move current_ptr to head of the list
do
{
temp_ptr = current_ptr->next; // set temp pointer to point
// to the remainder of the list
delete current_ptr; // delete current
current_ptr = temp_ptr;
} while (temp_ptr != NULL);
}
-
December 10th, 2010, 09:44 PM
#2
Re: C++ Linked List Issues
 Originally Posted by gpjacks
I am having trouble with my linked list. My teacher wants us to add the total number of nodes, the total from the user input (example: if the user enters 5, 5, ,5 she wants the results displayed as 15. As far as counting the nodes I have no idea how to implement it inside my code
First, use code tags when posting code. The code you posted is practically unreadable,
Second -- I'm curious:
You're taking a course in C++ programming, and you are given a linked list to code? What is the name of the course you're taking? Is it data structures? If it isn't, why in the world would you be given such an assignment, and to do it in C++ no less?
Third, if you were given a linked list to code, why not start out coding a linked list class so that the code is encapsulated? Right now, you're maintaining this list all over the place instead of all of the code consolidated to a single class. That is the main problem with your code -- no one who would write a linked list in this manner, unless you are writing in 'C', not C++. Another problem in your code is that you didn't delete the list at the end. This is one reason why a class is the way to do this -- in your destructor, you would automatically clean up the linked list.
Also, if you wrote this code, why wouldn't you have an idea where to count the nodes? If you wrote the code, then you would know how and when a node is added, therefore increase a count by one. Wherever you add a node, you increase a count variable. It doesn't matter what the person entered as input -- each time you add a single node, you add 1 to a counter.
Since there is nothing Visual C++ specific about your code, I will mention that most professional C++ programmers use the std::list class for linked lists and do not resort to coding their own. So to be honest, your code, if written by one of us, would be less than 5 to 10 lines of code, since we would use what the C++ library gives us.
Regards,
Paul McKenzie
Last edited by Paul McKenzie; December 11th, 2010 at 01:18 AM.
-
December 11th, 2010, 12:23 AM
#3
Re: C++ Linked List Issues
Sorry about the code tags. Im new here. Yes, I wrote the code. The class is called "Computer Science C++ by Knowlton and Hunt. I know you guys can write this code faster than I can because you are experts and I am a beginner. That's why I am asking for help.
This is the example we are going off of "counties.cpp"
Code:
// counties.cpp
// example of a dynamically-allocated linked list
//include files
#include <iostream>
#include <iomanip>
#include <string>
using namespace std;
// global structure definitions, variables, and constants
struct county_node //node for linked list
{
string county_name;
long population;
county_node *next;
};
county_node *head_ptr; // pointer to head of linked list
county_node *current_ptr; // pointer to current node
// function prototypes
int get_county_data (string &name, long &popul);
void add_node (string &name, long popul);
void move_current_to_end();
void display_list();
void delete_list();
// beginning of main function
int main()
{
string name;
long popul;
if (get_county_data(name, popul)) // prompt user for data for the node
{
head_ptr = new county_node; // initialize list head
head_ptr->county_name = name;
head_ptr->population = popul;
head_ptr->next = NULL; // initialize next node pointer to NULL
while (get_county_data(name, popul))
{
add_node (name, popul);
}
display_list(); // display output
delete_list(); // free the memory used by the linked list
}
return 0;
}
// function that gets the data from the user
int get_county_data(string &name, long &popul)
{
int keep_data = 1;
cout << "Enter country name (press Enter alone to stop): ";
getline (cin, name);
if (name != "")
{
cout << "Enter country population: ";
cin >> popul;
cin.ignore (80, '\n');
}
else
{
keep_data = 0;
}
return (keep_data);
}
// function that adds a node to the end on the linked list.
void add_node (string &name, long popul)
{
county_node *new_rec_ptr; // declare local pointer for new node
new_rec_ptr = new county_node; // allocate memory for a new node and
// initialize pointer to point to it
new_rec_ptr->county_name = name;
new_rec_ptr->population = popul;
new_rec_ptr->next = NULL; // set next pointer of new node to NULL
move_current_to_end(); // make sure current_ptr is at end of line
current_ptr->next = new_rec_ptr; // place new node at end of the list
}
// function that moves current_ptr to end of the linked list
void move_current_to_end()
{
current_ptr = head_ptr; // move current_ptr to head of the list
while (current_ptr->next!= NULL)
{ // transverse list until NULL is reached
current_ptr = current_ptr->next;
}
}
// function that displays entire linked list
void display_list()
{
current_ptr = head_ptr; // move current_ptr to head of list
cout << "County" << " ----------------------- " << " Population\n";
do
{
cout.setf(ios::left);
cout << setw(25) << current_ptr->county_name;
cout.setf(ios::right);
cout << setw (12) << current_ptr->population << endl;
current_ptr = current_ptr->next; // point current_ptr to next node
} while (current_ptr != NULL); // loop until end of list
}
// function that frees the memory used by the link list
void delete_list()
{
county_node *temp_ptr; // pointer used for temporary storage
current_ptr = head_ptr; // move current_ptr to head of the list
do // transverse list
{
temp_ptr = current_ptr->next; // set temp pointer to point
// to the remainder of the list
delete current_ptr; // delete current
current_ptr = temp_ptr;
} while (temp_ptr != NULL);
}
Here is my code in tags so its readable.
Code:
/*
1. Write a program that allows the user to enter a series of positive
floating-point numbers.
2. Store the numbers in a linked list.
3. Allow the user to enter numbers until a negative number is entered. (terminate when a negative # is entered).
4. Next, transverse the list to total the numbers entered, number of nodes, and average to the screen
Ok so now I have some numbers coming up at least, but now I need to transverse the list to total the numbers, count the number of
nodes, and average to the screen. Here is my revised code.
*/
#include <iostream>
#include <iomanip>
using namespace std;
struct node
{
float value;
node *next;
};
node *head_ptr = NULL;
node *current_ptr = NULL;
int get_user_data ();
void add_node (float numbers);
void move_current_to_end();
void display_list();
void delete_list();
int main()
{
float numbers;
while(get_user_data());
display_list(); // display the countries and population
return 0;
}
int get_user_data ()
{
float input = 1;
if (input != 0)
{ // Display details for what temp points to
cout << "Please insert a number: " << endl;
cin >> input;
add_node(input);
cout << endl; // Blank line
}
else
{
input = 0;
}
return (input);
}
void add_node (float numbers)
{
node *temp, *temp2; // declare local pointer for new node
temp = new node; // allocate memory for a new node and
// initialize pointer to point to it
temp->value = numbers;
temp->next = NULL; //new node ptr to null (end of list)
if(head_ptr == NULL)
head_ptr = temp;
else
{
//traverse list and find the end
temp2 = head_ptr; //start at head of list
while (temp2->next != NULL)
{
temp2 = temp2->next; //advance to next node
}
temp2->next = temp; //found the end, append temp (the new node)
}
}
void move_current_to_end()
{
current_ptr = head_ptr; // move current_ptr to head of the list
while (current_ptr->next!= NULL)
{
// transverse list until NULL is reached
current_ptr = current_ptr->next;
}
}
void display_list()
{
int count = 0;
current_ptr = head_ptr; // move current_ptr to head of list
cout << "Numbers" << endl;;
do
{
cout.setf(ios::left);
cout << setw(5) << current_ptr->value << " - ";
current_ptr = current_ptr->next; // point current_ptr to next node
} while (current_ptr != NULL); // loop until end of list
cout<<endl;
}
void delete_list()
{
node *temp_ptr; // pointer used for temporary storage
current_ptr = head_ptr; // move current_ptr to head of the list
do // transverse list
{
temp_ptr = current_ptr->next; // set temp pointer to point
// to the remainder of the list
delete current_ptr; // delete current
current_ptr = temp_ptr;
} while (temp_ptr != NULL);
}
-
December 11th, 2010, 01:35 AM
#4
Re: C++ Linked List Issues
 Originally Posted by gpjacks
Sorry about the code tags. Im new here. Yes, I wrote the code. The class is called "Computer Science C++ by Knowlton and Hunt. I know you guys can write this code faster than I can because you are experts and I am a beginner.
My point is that we would use std::list, and not code our own linked list.
Code:
#include <list>
#include <iostream>
int main()
{
std::list<float> MyList;
MyList.push_back( 10 );
MyList.push_back( 20 );
std::cout << 'The number of items in the list is " << MyList.size();
}
I just added two float items to a linked list, and outputted how many I added.
4. Next, transverse the list to total the numbers entered, number of nodes, and average to the screen
Write a function to do this.
Code:
int CountNodes( )
{
int numberOfNodes = 0;
if ( I do not have a head node )
return 0;
point current node to head node
while ( current node is valid )
add 1 to numberOfNodes
make current node = the next node of current node
end the while loop
return numberOfNodes;
}
Then you call this from anywhere. That is how you figure out where to place the code -- you don't place it anywere -- you create a function, and it doesn't matter where it's placed, as it's invoked when you call it.
Secondly, all you're doing in that function is starting from the head node, and then going to the next node until you've reached the end. For each node you're incrementing a counter. Then just return the counter at the end.
Regards,
Paul McKenzie
-
December 11th, 2010, 03:23 PM
#5
Re: C++ Linked List Issues
So, something more like this? Thanks for the help by the way!
Code:
/*
1. Write a program that allows the user to enter a series of positive
floating-point numbers.
2. Store the numbers in a linked list.
3. Allow the user to enter numbers until a negative number is entered. (terminate when a negative # is entered).
4. Next, transverse the list to total the numbers entered, number of nodes, and average to the screen
Ok so now I have some numbers coming up at least, but now I need to transverse the list to total the numbers, count the number of
nodes, and average to the screen. Here is my revised code.
*/
#include <iostream>
#include <iomanip>
using namespace std;
struct node
{
float value;
node *next;
};
node *head_ptr = NULL;
node *current_ptr = NULL;
int get_user_data ();
void add_node (float numbers);
void move_current_to_end();
int CountNodes( );
void display_list();
void delete_list();
int main()
{
while(get_user_data());
display_list(); // display the countries and population
return 0;
}
int get_user_data ()
{
float input = 1;
if (input != 0)
{ // Display details for what temp points to
cout << "Please insert a number: " << endl;
cin >> input;
add_node(input);
cout << endl; // Blank line
}
else
{
input = 0;
}
return (input);
}
void add_node (float numbers)
{
node *temp, *temp2; // declare local pointer for new node
temp = new node; // allocate memory for a new node and
// initialize pointer to point to it
temp->value = numbers;
temp->next = NULL; //new node ptr to null (end of list)
if(head_ptr == NULL)
head_ptr = temp;
else
{
//traverse list and find the end
temp2 = head_ptr; //start at head of list
while (temp2->next != NULL)
{
temp2 = temp2->next; //advance to next node
}
temp2->next = temp; //found the end, append temp (the new node)
}
}
void move_current_to_end()
{
current_ptr = head_ptr; // move current_ptr to head of the list
while (current_ptr->next!= NULL)
{
// transverse list until NULL is reached
current_ptr = current_ptr->next;
}
}
void display_list()
{
current_ptr = head_ptr; // move current_ptr to head of list
cout << "Numbers" << endl;;
do
{
cout.setf(ios::left);
cout << setw(5) << current_ptr->value << " - " << endl << endl;
current_ptr = current_ptr->next; // point current_ptr to next node
}while (current_ptr != NULL); // loop until end of list
cout << "Number of Nodes: "; // calculates the number of nodes
cout << endl << endl;
cout << CountNodes();
cout << endl << endl;
}
// function for calculating the amount of nodes
int CountNodes( )
{
int numberOfNodes = 0;
current_ptr = head_ptr;
if(head_ptr == NULL)
return 0;
while ( current_ptr != NULL )
{
numberOfNodes ++;
current_ptr = current_ptr->next;
}
return numberOfNodes;
}
void delete_list()
{
node *temp_ptr; // pointer used for temporary storage
current_ptr = head_ptr; // move current_ptr to head of the list
do // transverse list
{
temp_ptr = current_ptr->next; // set temp pointer to point
// to the remainder of the list
delete current_ptr; // delete current
current_ptr = temp_ptr;
} while (temp_ptr != NULL);
}
-
December 13th, 2010, 03:01 PM
#6
Re: C++ Linked List Issues
can you please post the corrected linked list issue? I tried this one, but it didn't work.
Thanks,
Charlote
-
December 13th, 2010, 03:57 PM
#7
Re: C++ Linked List Issues
 Originally Posted by charlote
can you please post the corrected linked list issue? I tried this one, but it didn't work.
Thanks,
Charlote
Be aware that teachers do visit forums like CG to sort out the homework cheaters.
-
December 13th, 2010, 11:46 PM
#8
Re: C++ Linked List Issues
I got it all figured out. Thanks for your help. And no I wasn't cheating I was asking for help.
-
December 14th, 2010, 05:37 AM
#9
Re: C++ Linked List Issues
 Originally Posted by gpjacks
So, something more like this? Thanks for the help by the way!
Code:
/*
1. Write a program that allows the user to enter a series of positive
floating-point numbers.
2. Store the numbers in a linked list.
3. Allow the user to enter numbers until a negative number is entered. (terminate when a negative # is entered).
4. Next, transverse the list to total the numbers entered, number of nodes, and average to the screen
Ok so now I have some numbers coming up at least, but now I need to transverse the list to total the numbers, count the number of
nodes, and average to the screen. Here is my revised code.
*/
#include <iostream>
#include <iomanip>
using namespace std;
struct node
{
float value;
node *next;
};
node *head_ptr = NULL;
node *current_ptr = NULL;
int get_user_data ();
void add_node (float numbers);
void move_current_to_end();
int CountNodes( );
void display_list();
void delete_list();
int main()
{
while(get_user_data());
display_list(); // display the countries and population
return 0;
}
int get_user_data ()
{
float input = 1;
if (input != 0)
{ // Display details for what temp points to
cout << "Please insert a number: " << endl;
cin >> input;
add_node(input);
cout << endl; // Blank line
}
else
{
input = 0;
}
return (input);
}
void add_node (float numbers)
{
node *temp, *temp2; // declare local pointer for new node
temp = new node; // allocate memory for a new node and
// initialize pointer to point to it
temp->value = numbers;
temp->next = NULL; //new node ptr to null (end of list)
if(head_ptr == NULL)
head_ptr = temp;
else
{
//traverse list and find the end
temp2 = head_ptr; //start at head of list
while (temp2->next != NULL)
{
temp2 = temp2->next; //advance to next node
}
temp2->next = temp; //found the end, append temp (the new node)
}
}
void move_current_to_end()
{
current_ptr = head_ptr; // move current_ptr to head of the list
while (current_ptr->next!= NULL)
{
// transverse list until NULL is reached
current_ptr = current_ptr->next;
}
}
void display_list()
{
current_ptr = head_ptr; // move current_ptr to head of list
cout << "Numbers" << endl;;
do
{
cout.setf(ios::left);
cout << setw(5) << current_ptr->value << " - " << endl << endl;
current_ptr = current_ptr->next; // point current_ptr to next node
}while (current_ptr != NULL); // loop until end of list
cout << "Number of Nodes: "; // calculates the number of nodes
cout << endl << endl;
cout << CountNodes();
cout << endl << endl;
}
// function for calculating the amount of nodes
int CountNodes( )
{
int numberOfNodes = 0;
current_ptr = head_ptr;
if(head_ptr == NULL)
return 0;
while ( current_ptr != NULL )
{
numberOfNodes ++;
current_ptr = current_ptr->next;
}
return numberOfNodes;
}
void delete_list()
{
node *temp_ptr; // pointer used for temporary storage
current_ptr = head_ptr; // move current_ptr to head of the list
do // transverse list
{
temp_ptr = current_ptr->next; // set temp pointer to point
// to the remainder of the list
delete current_ptr; // delete current
current_ptr = temp_ptr;
} while (temp_ptr != NULL);
}
As far as I could see from the assignment there is no requirement to use a self-written linked-list. Hence you should follow the advice of Paul and use std::list rather than handling the linked-list operations with global pointers yourself:
Code:
#include <iostream>
#include <iomanip>
#include <list>
using namespace std;
// remove struct node, head_ptr and current_ptr
// add the list as argument to the functions
int get_user_data (std::list<float>& mylist);
void add_node (std::list<float>& mylist, float numbers);
// void move_current_to_end();
int countNodes( const std::list<float>& mylist);
void display_list( const std::list<float>& mylist);
void delete_list(std::list<float>& mylist);
Note, it would be better you would put all the functions to a class rather than to have global functions. But by using std::list you at least must not use code for implementing basic linked-list operations. Those functions always must be encapsulated even in C code.
If you have done so far you could rewrite your main function like
Code:
int main()
{
std::list<float> mylist;
while(get_user_data(mylist));
display_list(mylist); // display the values and calculations
return 0;
}
-
December 14th, 2010, 11:36 AM
#10
Re: C++ Linked List Issues
Thank you for showing me the syntax and pointing out a few things. Now that the semester is over I learn how to go it like that. It seems a lot easier than the way I did it, but it was how the book taught us and it had nothing in it about std::list or anything like that. It was a pretty old book. Well, anyway thanks again for the help and the pointers (no pun intended) Happy Holidays!
-
December 23rd, 2010, 06:45 PM
#11
Re: C++ Linked List Issues
My post was not for you gpjacks, it was for charlote. CG welcome people that put an effort in writing code themselves but need a helping hand.
Tags for this Thread
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|