correct behavour of at()

I wanted to build this list ground up again. I'd like to know if I'm on a better track this time?

So far, my example really only tests Iter creation/copying/assigning.


1) Is my Iter assignment operator correct?

2) Is this how the funtion at() is normally implemented?

AList.h:

Code:

#ifndef ALIST_H
#define ALIST_H

template <typename T>
class List{
    
    private:
      
        struct Node{
            
            T data;
            Node *next, *prev;
            Node(const T& d, Node* n = 0, Node* p = 0) : data(d), next(n), prev(p) {};
            Node(const Node& src) : data(src.data), next(src.next), prev(src.prev) {};
        };
        
    Node *head, *tail;    
        
    unsigned int sizeOfList;
        
    public:
        
        class Iter{
            
            private:
                
                Node* curr;
                
            public:
                
                Iter(Node* c = 0) : curr(c) {};
                Iter(const Iter& rhs_i) : curr(rhs_i.curr) {};
                
                Iter& operator=(const Iter& rhs_i){ Node* c = rhs_i.curr; return *this; };
                
                const T& operator*() const { return curr->data; };
                T& operator*() { return curr->data; };
                
                bool operator!=(const Iter& i) { return i.curr != curr; };
                bool operator==(const Iter& i) { return i.curr == curr; };
                
                Iter& operator++(int) { curr = curr->next; return *this; };
                Iter& operator--(int) { curr = curr->prev; return *this; };
                
        };
        
    List<T>();
    List<T>(const List& src);
    unsigned int size(){ return sizeOfList; };
    
    const Iter begin() const { return Iter(head); };
    const Iter end() const { return Iter(0); };
    const Iter rbegin() const { return Iter(tail); };
    const Iter rend() const { return Iter(0); };
    
    
    /*  Here's at()
    */
    const Iter at(int n) const {
    
    	Iter temp = begin();
			for(int i = 0;  i < n; i++){
				
				temp++;
			}
			
			return temp;
        }
        
};

template <typename T>
List<T>::List() : head(0), tail(0), sizeOfList(0) {}

#endif

AMain.cpp:

Code:

#include <iostream>
#include "AList.h"

int main(){
    
    List<int> mlist;
    
    List<int>::Iter i;
    List<int>::Iter j(i);
    List<int>::Iter k;
    List<int>::Iter l = k = j;
    
    return 0;
}

Code:

 Iter& operator=(const Iter& rhs_i){ Node* c = rhs_i.curr; return *this; };

You are assigning to a temporary.

I don't think either Node nor Iter needs a special copy contructor or assignment operator. They don't require anything else but the shallow copy that the compiler would produce automatically.

You have incorrectly implemented postincrement as if it were preincrement, so the semantics of it is wrong.

There should be no "at" function for a list. Any O(n) running time list accessor is fundamentally wrong, as it encourages writing horribly inefficient code. This is why the SC++L does not have it for a list.

I think Node should be noncopyable. It doesn't make sense to copy nodes.

Originally Posted by anon

I don't think either Node nor Iter needs a special copy contructor or assignment operator. They don't require anything else but the shallow copy that the compiler would produce automatically.

Originally Posted by iMalc

There should be no "at" function for a list. Any O(n) running time list accessor is fundamentally wrong, as it encourages writing horribly inefficient code. This is why the SC++L does not have it for a list.

So on both accounts just remove and implement w/out?

Originally Posted by CornedBee

I think Node should be noncopyable. It doesn't make sense to copy nodes.

Is there a special way of preventing the copying of Nodes?

Make the copy constructor and copy assignment operator private.

Originally Posted by sh3rpa

So on both accounts just remove and implement w/out?

yep, yep.

AAAck.. I would not provide an at() function for a list. You run the risk of somebody attempting to use it!

EDIT: Seems I was beaten to the punch.

Originally Posted by laserlight

Make the copy constructor and copy assignment operator private.

And don't implement them.


Alternatively, derive from boost::noncopyable.