Reversing Linked List

[Snaksa]

Hi everybody. I'm trying to reverse Linked List but I face some difficulties. I'm using two lists. The first one is the one that I want to reverse. The second list is for help. It's something like stack. When I print the result of the second list, it outputs the reversed elements of the first list but then it also inputs the first list again. I think something is wrong with my loop but.. :/
Here are the two functions that I use:

  template<class T>  
    void List<T>::Insert_After(List *temp1, T x)
      {
        List *temp2 = new List;
        *temp2 = *temp1; 
        temp2->data = x;
        temp2->next = temp1->next;
        temp1->next = temp2;   
      }

template<class T>   
    void List<T>::Reverse()
      {
        
        List *temp2;
        temp2 = start_ptr;
        B.start_ptr = temp2;
        temp2 = temp2->next;
          while(temp2 != NULL) 
            {
              B.Insert_Before(B.start_ptr, temp2->data);
              temp2 = temp2->next;
            }     
      B.Print_List();
      }

I have two Lists A and B.

For example:
If the elements of list A are:

A
B
C
D

The output is:

D
C
B
A
B
C
D

Can someone help me please? :/

[keskiverto]

Show definition of List, please.

[Snaksa]

Here is my whole code:

    #include<iostream>
    #include<string>
    using namespace std;
    
    template<class T>
    class List
      {
        private:
        T data;
        List *next;
        List *start_ptr;
        public:
        List();
        void Add_node(List *temp1);
        void Insert(T x);
        void Insert_After(List *temp1, T x);
        void Insert_Before(List *temp1, T x);
        void Reverse();
        void Print_List();  
      };
    List<char> A;
    List<char> B;
      
    template<class T>
    List<T>::List()
      {
        start_ptr = NULL;          
      }
    
    template<class T>
    void List<T>::Insert(T x)
      {
       List *temp1 = new List;
       temp1->data = x; 
       A.Add_node(temp1);              
      }
    
    template<class T>
    void List<T>::Add_node(List *temp1)
      {
        List *temp2;
        temp1->next = NULL;
        if(start_ptr == NULL) start_ptr = temp1;
        else
          {
            temp2 = start_ptr;
            while(temp2->next != NULL)
              temp2 = temp2->next;  
            temp2->next = temp1;         
          }      
      }
     
    template<class T>
    void List<T>::Insert_Before(List *temp1, T x)
      {
        List *temp2 = new List;
        *temp2 = *temp1;
        temp1->data = x;
        temp1->next = temp2; 
      }
    
    template<class T>  
    void List<T>::Insert_After(List *temp1, T x)
      {
        List *temp2 = new List;
        *temp2 = *temp1; 
        temp2->data = x;
        temp2->next = temp1->next;
        temp1->next = temp2;   
      }
    
    template<class T>   
    void List<T>::Reverse()
      {
        
        List *temp2;
        temp2 = start_ptr;
        B.start_ptr = temp2;
        temp2 = temp2->next;
          while(temp2 != NULL) 
            {
              B.Insert_Before(B.start_ptr, temp2->data);
              temp2 = temp2->next;
            }     
      B.Print_List();
      }
    
    template<class T>  
    void List<T>::Print_List()
      {
        List *temp1;
        temp1 = start_ptr;
        if(temp1 == NULL) cout << "Empty List!" << endl;
        else
          {
            while(temp1 != NULL)
              {
                cout << temp1->data << endl;
                temp1 = temp1->next;          
              }     
          }                     
      }

    int main()
      {
        for(int i = 'A'; i <= 'D'; i++)
          A.Insert(i);
        A.Print_List();
        cout << endl;
        A.Reverse();
        system("pause");
        return 0;        
      }

[vlad from moscow]

template<class T>  
void List<T>::Insert_After(List *temp1, T x)
{
        List *temp2 = new List;
        temp2->data = x;
        temp2->next = temp1->next;
        temp1->next = temp2;   
}

In the function above I removed statement

*temp2 = *temp1;

because I do not see any sense of this statement.

As for the function below then because I do not know the definition of class List its code can be wrong. Nevertheless...

template<class T>   
void List<T>::Reverse()
{
        B.start_ptr = 0;

        List *next = start_ptr;

        while ( next != NULL ) 
        {
              List *temp = new List;
              temp->data = next->data;
              temp->next = B.start;
              B.start = temp;
              next = next->next;
         }     
         B.Print_List();
      }

[vlad from moscow]

The design of your list is very bad. Why does each node have the data member List *start_ptr;?
It is much better to decclare internal structure Node inside your class List with one constructor. For example

struct Node
{
   Node *next;
   T data;
   Node( const T &data, Node *next = nullptr ) : data( data ), next( next ) {}
} *Head;

[Snaksa]

The design of your list is very bad. Why does each node have the data member List *start_ptr;?
It is much better to decclare internal structure Node inside your class List with one constructor. For example

I tried to do whay you said but I get an error when I declare new variables in the other functions. For example on the 33 roll in my code I get this:

missing template arguments before '*' token

This is my structure:

template<class T>
    struct Node
      {
        T data;
        Node *next;    
      };

The start_pointer is still in the List definition. Should I move it too?

[Lowest0ne]

@ vlad: The node constructor should not take T by reference.

@ Snaksa: I agree with vlad though, in context:

class List
{
private:
  Node
  {
    Node* next;
    T data;
    Node ( /* ... */ ) 
  } * head;
public:
  /* ... */
};

The output you printed means that something is very wrong with your methods. You succesfully reversed the list, but the old head's next is not null. This wouldn't be so bad, but the fact that the output isnt an infinate loop means that the D in the beginning of the list is not the D at the end of the list.

Forget list1 and list2:

class List
{
  /* ... */
};

int main()
{
  List myList;
  /* ... */
}

[vlad from moscow]

@LowestOne

@ vlad: The node constructor should not take T by reference.

Why should not it take T by reference?

[Snaksa]

So you don't know how I can solve the problem? :/

[Snaksa]

#Solved