#include <iostream.h>
struct node
{
int id;
node *next;
};
node *head = NULL;
node *curr;
void add_head( int id )
{
curr = new node;
curr->next = head;
head = curr;
head->id = id;
}
int main()
{
int i, nodes;
cout << "\nenter node count::";
cin >> nodes;
for( i = -1; i < nodes; ++i )
{
add_head( i );
}
cout << "\nlist dump..";
curr = head;
while( curr->next != NULL )
{
cout << "\ncurr->id = " << curr->id;
curr = curr->next;
}
cout << "\n\n";
return( 0 );
}
will some one please review my source
Author
this is my source code for a single linked list. I think I''m doing this right, but I would like some feed back from you people.
Once I get linked list down, I''ll be set. So any help and advice (if its''t mean) would be nice.
1) What is "iostream.h" doing? No code written in the year 2002 should mention it.
2) Read this to save me from having to explain why your list''s interface bites. In simple terms, the users of your list should not have to concern themselves with managing the nodes of your list.
2) Read this to save me from having to explain why your list''s interface bites. In simple terms, the users of your list should not have to concern themselves with managing the nodes of your list.
You got the basic functionality of the linked list correctly. However :
#include <iostream.h>
That''s very bad, use <iostream> instead. Follow it with using namespace std; if you want to write cin, cout, endl and so on instead of std::cin, std::cout, std::endl...
(iostream.h is deprecated and should not be used in new code. It is only kept for compatibility with old pre-standard code).
void add_head( int id )
It works, but you should have a reference to head as a parameter to let you have multiple linked lists. And declare curr as a local variable, since the rest of the program shouldn''t know about it.
int main()
Idem in main(), curr should be a local variable.
You''re never deleting your list, nor providing a way to do it, that''s a memory leak, that''s very bad.
And no, I''m not being mean
Documents [ GDNet | MSDN | STL | OpenGL | Formats | RTFM | Asking Smart Questions ]
C++ Stuff [ MinGW | Loki | SDL | Boost. | STLport | FLTK | ACCU Recommended Books ]
#include <iostream.h>
That''s very bad, use <iostream> instead. Follow it with using namespace std; if you want to write cin, cout, endl and so on instead of std::cin, std::cout, std::endl...
(iostream.h is deprecated and should not be used in new code. It is only kept for compatibility with old pre-standard code).
void add_head( int id )
It works, but you should have a reference to head as a parameter to let you have multiple linked lists. And declare curr as a local variable, since the rest of the program shouldn''t know about it.
int main()
Idem in main(), curr should be a local variable.
You''re never deleting your list, nor providing a way to do it, that''s a memory leak, that''s very bad.
And no, I''m not being mean
Documents [ GDNet | MSDN | STL | OpenGL | Formats | RTFM | Asking Smart Questions ]
C++ Stuff [ MinGW | Loki | SDL | Boost. | STLport | FLTK | ACCU Recommended Books ]
Author
Eh, I was just trying to get a working linked list. I don''t care about global and local issues and any of that other stuff. I''m just trying to actually implement a linked list. I''m just playing with some code. It dosn''t need to be all perfect.
Any ways, its a start on a linked list. I want to know if its working correctly or not. I have a user enter a value for the program. Its suppose to add head node and travers throught the list. This program is a test to see if I got it right. Thats why theres a input for the amount of nodes. I want to control that amount. I''m just starting on linked list again. Last time I didn''t even get half this far, so I think I''m doin pretty good.
Any ways, its a start on a linked list. I want to know if its working correctly or not. I have a user enter a value for the program. Its suppose to add head node and travers throught the list. This program is a test to see if I got it right. Thats why theres a input for the amount of nodes. I want to control that amount. I''m just starting on linked list again. Last time I didn''t even get half this far, so I think I''m doin pretty good.
Well, as I said, the basic functionality of your list works, we''re pointing at how it could be be better.
Honestly, until you start dealing with arbitrary insertion (not just at the beginning), and removal of nodes, there is just no way we can tell you wether it is good or bad... there is not enough to base a judgement on.
Documents [ GDNet | MSDN | STL | OpenGL | Formats | RTFM | Asking Smart Questions ]
C++ Stuff [ MinGW | Loki | SDL | Boost. | STLport | FLTK | ACCU Recommended Books ]
Honestly, until you start dealing with arbitrary insertion (not just at the beginning), and removal of nodes, there is just no way we can tell you wether it is good or bad... there is not enough to base a judgement on.
Documents [ GDNet | MSDN | STL | OpenGL | Formats | RTFM | Asking Smart Questions ]
C++ Stuff [ MinGW | Loki | SDL | Boost. | STLport | FLTK | ACCU Recommended Books ]
Now try making a simple user interface for your linked list with these components.
User Interface:
------------------
Create Linked List
Add Node
Delete Node
Move to Head of Link List
Move to Next node
Destroy List
Good Luck!!
User Interface:
------------------
Create Linked List
Add Node
Delete Node
Move to Head of Link List
Move to Next node
Destroy List
Good Luck!!
quote:Original post by sakky
Eh, I was just trying to get a working linked list. I don''t care about global and local issues and any of that other stuff. I''m just trying to actually implement a linked list. I''m just playing with some code. It dosn''t need to be all perfect.
If you''re going to get all defensive and uptight when people offer the criticism that you asked for, why bother asking for it in the first place.
But, fine.
Your code is simply wonderful. Good job. Long may you continue to produce such high quality programming.
quote:Any ways, its a start on a linked list. I want to know if its working correctly or not.
No, it isn''t, because it''s making the user deal with nodes. I don''t want to deal with nodes! I have enough to worry about with taking care of my own objects. The list''s objects should not be my concern.
quote:I have a user enter a value for the program. Its suppose to add head node and travers throught the list. This program is a test to see if I got it right. Thats why theres a input for the amount of nodes. I want to control that amount. I''m just starting on linked list again. Last time I didn''t even get half this far, so I think I''m doin pretty good.
Read the article I linked. Certain design decisions -- in particular, the use of sentinel nodes -- will make the implementation of the list easier than would be the case with the kind of list you''re writing. Yes, I know it''s another idea for you to think about, but at the end of the day, I think it''ll be worth it. When you have sentinel nodes, certain operations become magically easier, as you no longer have to special-case anyway.
Hey, now that''s a good article! A very good overview of implementing linked lists.
Thank DrPizza
Thank DrPizza
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement