review my linked list

This is a discussion on review my linked list within the C++ Programming forums, part of the General Programming Boards category; Please reply with your comments. Code: #ifndef linkedlist_h #define string_h #include<iostream> using namespace std; int LLstrlen(char[]); int LLstrlen(char str[]) { ...

  1. #1
    Registered User Mr_Jack's Avatar
    Join Date
    Oct 2003
    Posts
    63

    review my linked list

    Please reply with your comments.

    Code:
    #ifndef linkedlist_h
    #define string_h
    
    #include<iostream>
    using namespace std;
    
    int LLstrlen(char[]);
    int LLstrlen(char str[])
    {
    	int i = 0;
    	while(str[i] != '\0')
    		i++;
    	return i;
    }
    
    class node
    {
    private:
    	int elementNum;
    	node * next;
    	node * prev;
    	char ch;
    public:
    	friend class string;
    	operator=(char);
    	node();
    	friend ostream& operator<<(ostream&, const node&);
    };
    node::node()
    {
    	next = 0;
    	prev = 0;
    }
    node::operator=(char c)
    {
    	this->ch = c;
    }
    ostream& operator<<(ostream & os, const node & n)
    {
    	os << n.ch;
    	return os;
    }
    
    class linkedlist
    {
    private:
    	node * head;
    	node * current;
    	node * tail;
    	int len;
    public:
    	friend ostream& operator<<(ostream&, linkedlist&);
    	linkedlist();
    	void clearList();
    	void calcLen();
    	int getLen();
    	void createNext();
    	void gotoNext();
    	void gotoPrev();
    	void gotoHead();
    	void gotoTail();
    	void gotoElement(int);
    	void setCurCh(char);
    	char getCurCh()const;
    	void setCurElementNum(int);
    	char * createlinkedlist();
    	operator=(char[]);
    	node operator[](int);
    	friend void operator>>(istream&, linkedlist&);
    };
    node linkedlist::operator[](int e)
    {
    	gotoElement(e);
    	return *current;
    }
    ostream& operator<<(ostream & os, linkedlist & ll)
    {
    	ll.gotoHead();
    	for(int i = 0; i < ll.len; i++)
    	{
    		os << ll.getCurCh();
    		ll.gotoNext();
    	}
    	return os;
    }
    linkedlist::operator=(char str[])
    {
    	clearList();
    	int len = LLstrlen(str);
    	for(int i = 0; i < len; i++)
    	{
    		setCurCh(str[i]);
    		setCurElementNum(i);
    		createNext();
    		gotoNext();
    	}
    	this->len = len;
    }
    void operator>>(istream& is, linkedlist& ll)
    {
    	ll.clearList();
    	char ch;
    	is.get(ch);
    	ll.setCurCh(ch);
    	ll.setCurElementNum(0);
    	int len = 0;
    	int i = 1;
    	while(ch != '\n')
    	{
    		is.get(ch);
    		ll.createNext();
    		ll.gotoNext();
    		ll.setCurCh(ch);
    		ll.setCurElementNum(i);
    		len++;
    		i++;
    	}
    	ll.len = len;
    }
    linkedlist::linkedlist()
    {
    	head = new node;
    	current = head;
    	len = 0;
    	tail = current;
    }
    void linkedlist::clearList()
    {
    	node * temp;
    	gotoTail();
    	for(int i = 0; i < len-1; i++)
    	{
    		temp = tail->prev;
    		delete current;
    		current = temp;
    	}
    	head = current;
    	tail = current;
    }
    void linkedlist::calcLen()
    {
    	if(len == 0)
    	{
    		while(current->next != 0)
    		{
    			len++;
    			current = current->next;
    		}
    	}
    }
    int linkedlist::getLen()
    {
    	return len;
    }
    void linkedlist::createNext()
    {
    	current->next = new node;
    	current->next->prev = current;
    }
    void linkedlist::gotoNext()
    {
    	current = current->next;
    }
    void linkedlist::gotoPrev()
    {
    	current = current->prev;
    }
    void linkedlist::gotoHead()
    {
    	current = head;
    }
    void linkedlist::gotoTail()
    {
    	current = tail;
    }
    void linkedlist::gotoElement(int n)
    {
    	gotoHead();
    	while(current->elementNum != n)
    		gotoNext();
    }
    void linkedlist::setCurCh(char c)
    {
    	current->ch = c;
    }
    void linkedlist::setCurElementNum(int n)
    {
    	current->elementNum = n;
    }
    char linkedlist::getCurCh()const
    {
    	return current->ch;
    }
    char * linkedlist::createlinkedlist()
    {
    	calcLen();
    	char * str = new char[len];
    	str[len] = '\0';
    	gotoHead();
    	for(int i = 0; i < len; i++)
    	{
    		str[i] = current->ch;
    		gotoNext();
    	}
    	return str;
    }
    #endif

  2. #2
    Senior Member joshdick's Avatar
    Join Date
    Nov 2002
    Location
    Phildelphia, PA
    Posts
    1,146
    Abstraction. The user of your linked list class shouldn't have to know about heads or tails or anything pointer related. A linked list class should act a lot like an array.

    Also, your class would be a lot more useful if it were a template. That way, you could store whatever you wanted in it.
    FAQ

    "The computer programmer is a creator of universes for which he alone is responsible. Universes of virtually unlimited complexity can be created in the form of computer programs." -- Joseph Weizenbaum.

    "If you cannot grok the overall structure of a program while taking a shower, you are not ready to code it." -- Richard Pattis.

  3. #3
    Veni Vidi Vice
    Join Date
    Aug 2001
    Posts
    343
    I totaly agree with joshdick. If I were to use your linked list I really donīt care how the underlying implementation looks like.I want a linked list with certain operations (independet of what datatype im using in it) that a linked list should support. Check this recent thread for more information. Also some suggesetions
    Why
    • define int LLstrlen(char[]); when u got strlen?
    • have a void linkedlist::calcLen()? Update linkedlist::len when you remove/add an element
    • friend class string; For what purpose? maybe friend class linkedlist?
    01000111011011110110111101100100 011101000110100001101001011011100110011101110011 01100100011011110110111001110100 01100011011011110110110101100101 01100101011000010111100101110011 0110100101101110 01101100011010010110011001100101
    Good things donīt come easy in life!!!

  4. #4
    Registered User Mr_Jack's Avatar
    Join Date
    Oct 2003
    Posts
    63
    The user doesn't need to know anything about the inner working of the class. I guess I should have made those gotoNext, gotoPrev functions private, since they are only used internally.
    The reason for making my own string len function is to see if I was able to do it effectively. calcLen is there for internal use, so I should have made it private.

    Thanks for your input!

  5. #5
    Code Goddess Prelude's Avatar
    Join Date
    Sep 2001
    Posts
    9,796
    Encapsulation is good, but a good container class shouldn't exactly be a black box. Let's take a look:

    >friend ostream& operator<<(ostream&, linkedlist&);
    Good, outputing a list with one little command is desirable.

    >linkedlist();
    Empty list construction, also good.

    >void clearList();
    This would be better named "clear". A linked list doesn't have to actually be implemented as a linked list. I've seen list containers that used binary trees as the underlying implementation. Keep your names short, easy to remember, and don't give away too much about the implementation so that it can be changed later if need be.

    >void calcLen();
    This is not needed. You can keep it private and have getLen call it if the length changed between calls to getLen. For example, a flag in the modification routines. If the flag is set, you assume that the length needs to be recalculated.

    >int getLen();
    This would be better named "size", or just "length".

    >void createNext();
    This can be made private and a more generic insertion routine added in its place for the client interface.

    >void gotoNext();
    This is good as an interface function. Though goto need not be there, you can simply use "next" and have the function return an iterator to the next item. Just as an aside, using a current iterator in your container isn't the best of ideas. It would be better to let the user create multiple iterators into the list to give them greater flexibility.

    >void gotoPrev();
    See above.

    >void gotoHead();
    Good, though "first" would be a better name.

    >void gotoTail();
    Good, though "last" would be a better name.

    >void gotoElement(int);
    Be wary of this kind of indexing operation. I wouldn't suggest including it unles you can make it efficient. Generally, this can be trivially done by the client the way you are doing it, so it really shouldn't be a part of the interface at all.

    >void setCurCh(char);
    >char getCurCh()const;
    Use iterators to access an item, this will work, but it is difficult to do well in a generic way.

    >void setCurElementNum(int);
    This isn't needed anywhere, it just adds fluff and bloat. Unless you have a good reason for adding bloat, don't.

    >char * createlinkedlist();
    Awful naming. This suggests you are creating a linked list, which violates encapsulation and lies about what it does. What it actually does is takes the list and makes a string out of it. A better name would be "tostring".

    >operator=(char[]);
    Good. An itnerface usually needs an assignment operator, but I notice a distressing lack of destructor and copy constructor.

    >node operator[](int);
    Be very careful with indexing, using the subscript operator in particular. Clients will think instinctively that it is as efficient as array indexing, which is just not possible with linked lists. The result will be the client using your class in a very inefficient manner.

    >friend void operator>>(istream&, linkedlist&);
    This is okay for your list of char, but for generic lists it doesn't make much sense and would be difficult to implement properly.
    My best code is written with the delete key.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. C++ Linked list program need help !!!
    By dcoll025 in forum C++ Programming
    Replies: 1
    Last Post: 04-20-2009, 11:03 AM
  2. Replies: 5
    Last Post: 11-04-2006, 06:39 PM
  3. Reverse function for linked list
    By Brigs76 in forum C++ Programming
    Replies: 1
    Last Post: 10-25-2006, 11:01 AM
  4. Template Class for Linked List
    By pecymanski in forum C++ Programming
    Replies: 2
    Last Post: 12-04-2001, 09:07 PM
  5. singly linked list
    By clarinetster in forum C Programming
    Replies: 2
    Last Post: 08-26-2001, 11:21 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21