[Martyr2]

Can we see your IntNode class code? It is really instrumental in see what you are doing with methods like getLink() and removeNodeAfter(). Also, just for an efficiency improvement, consider calling findMax() once, store the value and then use the value instead of calling findMax() each time. That causes the for loop in findMax() to loop through all nodes each time to find the same max value. Do it once and you will increase speed considerably.

So when we see what IntNode looks like, we can certainly help further.

[billinkw]

Of course, the IntNode class I am using is from my textbook. Thanks for the tip, i will assign the findMax output as a variable. Thank you.


Here it is:

/******************************************************************************
* An IntNode provides a node for a linked list with 
* integer data in each node.
*
* @note
*   Lists of nodes can be made of any length, limited only by the amount of
*   free memory in the heap. But beyond Integer.MAX_VALUE (2,147,483,647),
*   the answer from listLength is incorrect because of arithmetic
*   overflow. 
*
* @see
*   <A HREF="../../../../edu/colorado/nodes/IntNode.java">
*   Java Source Code for this class
*   (www.cs.colorado.edu/~main/edu/colorado/nodes/IntNode.java)</A>
*
* @author Michael Main 
*   <A HREF="mailto:main@colorado.edu"> (main@colorado.edu) </A>
*
* @version
*   March 6, 2002
*
* @see Node
* @see BooleanNode
* @see ByteNode
* @see CharNode
* @see DoubleNode
* @see FloatNode
* @see LongNode
* @see ShortNode
******************************************************************************/
public class IntNode
{
   // Invariant of the IntNode class:
   //   1. The node's integer data is in the instance variable data.
   //   2. For the final node of a list, the link part is null.
   //      Otherwise, the link part is a reference to the
   //      next node of the list.
   private int data;
   IntNode link;   


   /**
   * Initialize a node with a specified initial data and link to the next
   * node. Note that the initialLink may be the null reference, 
   * which indicates that the new node has nothing after it.
   * @param initialData
   *   the initial data of this new node
   * @param initialLink
   *   a reference to the node after this new node--this reference may be null
   *   to indicate that there is no node after this new node.
   * @postcondition
   *   This node contains the specified data and link to the next node.
   **/   
   public IntNode(int initialData, IntNode initialLink)
   {
      data = initialData;
      link = initialLink;
   }


   /**
   * Modification method to add a new node after this node.   
   * @param item
   *   the data to place in the new node
   * @postcondition
   *   A new node has been created and placed after this node.
   *   The data for the new node is item. Any other nodes
   *   that used to be after this node are now after the new node.
   * @exception OutOfMemoryError
   *   Indicates that there is insufficient memory for a new 
   *   IntNode. 
   **/
   public void addNodeAfter(int item)   
   {
      link = new IntNode(item, link);
   }          
   
   
   /**
   * Accessor method to get the data from this node.   
   * @param - none
   * @return
   *   the data from this node
   **/
   public int getData( )   
   {
      return data;
   }
   
   
   /**
   * Accessor method to get a reference to the next node after this node. 
   * @param - none
   * @return
   *   a reference to the node after this node (or the null reference if there
   *   is nothing after this node)
   **/
   public IntNode getLink( )
   {
      return link;                                               
   } 
    
    
   /**
   * Copy a list.
   * @param source
   *   the head of a linked list that will be copied (which may be
   *   an empty list in where source is null)
   * @return
   *   The method has made a copy of the linked list starting at 
   *   source. The return value is the head reference for the
   *   copy. 
   * @exception OutOfMemoryError
   *   Indicates that there is insufficient memory for the new list.   
   **/ 
   public static IntNode listCopy(IntNode source)
   {
      IntNode copyHead;
      IntNode copyTail;
      
      // Handle the special case of the empty list.
      if (source == null)
         return null;
         
      // Make the first node for the newly created list.
      copyHead = new IntNode(source.data, null);
      copyTail = copyHead;
      
      // Make the rest of the nodes for the newly created list.
      while (source.link != null)
      {
         source = source.link;
         copyTail.addNodeAfter(source.data);
         copyTail = copyTail.link;
      }
 
      // Return the head reference for the new list.
      return copyHead;
   }
   
   
   /**
   * Copy a list, returning both a head and tail reference for the copy.
   * @param source
   *   the head of a linked list that will be copied (which may be
   *   an empty list in where source is null)
   * @return
   *   The method has made a copy of the linked list starting at 
   *   source.  The return value is an
   *   array where the [0] element is a head reference for the copy and the [1]
   *   element is a tail reference for the copy.
   * @exception OutOfMemoryError
   *   Indicates that there is insufficient memory for the new list.   
   **/
   public static IntNode[ ] listCopyWithTail(IntNode source)
   {
      IntNode copyHead;
      IntNode copyTail;
      IntNode[ ] answer = new IntNode[2];
     
      // Handle the special case of the empty list.   
      if (source == null)
         return answer; // The answer has two null references .
      
      // Make the first node for the newly created list.
      copyHead = new IntNode(source.data, null);
      copyTail = copyHead;
      
      // Make the rest of the nodes for the newly created list.
      while (source.link != null)
      {
         source = source.link;
         copyTail.addNodeAfter(source.data);
         copyTail = copyTail.link;
      }
      
      // Return the head and tail references.
      answer[0] = copyHead;
      answer[1] = copyTail;
      return answer;
   }
   
   
   /**
   * Compute the number of nodes in a linked list.
   * @param head
   *   the head reference for a linked list (which may be an empty list
   *   with a null head)
   * @return
   *   the number of nodes in the list with the given head 
   * @note
   *   A wrong answer occurs for lists longer than Int.MAX_VALUE.     
   **/   
   public static int listLength(IntNode head)
   {
      IntNode cursor;
      int answer;
      
      answer = 0;
      for (cursor = head; cursor != null; cursor = cursor.link)
         answer++;
        
      return answer;
   }
   

   /**
   * Copy part of a list, providing a head and tail reference for the new copy. 
   * @param start/end
   *   references to two nodes of a linked list
   * @param copyHead/copyTail
   *   the method sets these to refer to the head and tail node of the new
   *   list that is created
   * @precondition
   *   start and end are non-null references to nodes
   *   on the same linked list,
   *   with the start node at or before the end node. 
   * @return
   *   The method has made a copy of the part of a linked list, from the
   *   specified start node to the specified end node. The return value is an
   *   array where the [0] component is a head reference for the copy and the
   *   [1] component is a tail reference for the copy.
   * @exception IllegalArgumentException
   *   Indicates that start and end are not references
   *   to nodes on the same list.
   * @exception NullPointerException
   *   Indicates that start is null.
   * @exception OutOfMemoryError
   *   Indicates that there is insufficient memory for the new list.    
   **/   
   public static IntNode[ ] listPart(IntNode start, IntNode end)
   {
      IntNode copyHead;
      IntNode copyTail;
      IntNode cursor;
      IntNode[ ] answer = new IntNode[2];
      
      // Make the first node for the newly created list. Notice that this will
      // cause a NullPointerException if start is null.
      copyHead = new IntNode(start.data, null);
      copyTail = copyHead;
      cursor = start;
      
      // Make the rest of the nodes for the newly created list.
      while (cursor != end)
      {
         cursor = cursor.link;
         if (cursor == null)
            throw new IllegalArgumentException
            ("end node was not found on the list");
         copyTail.addNodeAfter(cursor.data);
         copyTail = copyTail.link;
      }
      
      // Return the head and tail references
      answer[0] = copyHead;
      answer[1] = copyTail;
      return answer;
   }        
   
   
   /**
   * Find a node at a specified position in a linked list.
   * @param head
   *   the head reference for a linked list (which may be an empty list in
   *   which case the head is null)
   * @param position
   *   a node number
   * @precondition
   *   position > 0.
   * @return
   *   The return value is a reference to the node at the specified position in
   *   the list. (The head node is position 1, the next node is position 2, and
   *   so on.) If there is no such position (because the list is too short),
   *   then the null reference is returned.
   * @exception IllegalArgumentException
   *   Indicates that position is not positive.    
   **/   
   public static IntNode listPosition(IntNode head, int position)
   {
      IntNode cursor;
      int i;
      
      if (position <= 0)
           throw new IllegalArgumentException("position is not positive");
      
      cursor = head;
      for (i = 1; (i < position) && (cursor != null); i++)
         cursor = cursor.link;

      return cursor;
   }


   /**
   * Search for a particular piece of data in a linked list.
   * @param head
   *   the head reference for a linked list (which may be an empty list in
   *   which case the head is null)
   * @param target
   *   a piece of data to search for
   * @return
   *   The return value is a reference to the first node that contains the
   *   specified target. If there is no such node, the null reference is 
   *   returned.     
   **/   
   public static IntNode listSearch(IntNode head, int target)
   {
      IntNode cursor;
      
      for (cursor = head; cursor != null; cursor = cursor.link)
         if (target == cursor.data)
            return cursor;
        
      return null;
   }

   
   /**
   * Modification method to remove the node after this node.   
   * @param - none
   * @precondition
   *   This node must not be the tail node of the list.
   * @postcondition
   *   The node after this node has been removed from the linked list.
   *   If there were further nodes after that one, they are still
   *   present on the list.
   * @exception NullPointerException
   *   Indicates that this was the tail node of the list, so there is nothing
   *   after it to remove.
   **/
   public void removeNodeAfter( )   
   {
      link = link.link;
   }          
   
   
   /**
   * Modification method to set the data in this node.   
   * @param newData
   *   the new data to place in this node
   * @postcondition
   *   The data of this node has been set to newData.
   **/
   public void setData(int newData)   
   {
      data = newData;
   }                                                               
   
   
   /**
   * Modification method to set the link to the next node after this node.
   * @param newLink
   *   a reference to the node that should appear after this node in the linked
   *   list (or the null reference if there is no node after this node)
   * @postcondition
   *   The link to the node after this node has been set to newLink.
   *   Any other node (that used to be in this link) is no longer connected to
   *   this node.
   **/
   public void setLink(IntNode newLink)
   {                    
      link = newLink;
   }
   
   
}

[n8wxs]

In listSort() these 3 lines do not do what you think they do:

sorted = new IntNode(findMax(head), sorted);//Add it to the new list
head.removeNodeAfter();//remove it from the original
head = head.getLink();

findMax() finds a value somewhere in the list. The node it finds may or may not be the node removeNodeAfter() removes. There is no communication between findMax() and listSort() as to which node had the maximum value.

findMax() probably ought to return the node rather than the value.

The last line of the 3 is a problem because the link copied may be the null terminator.

This post has been edited by n8wxs: 24 October 2010 - 12:23 AM

[billinkw]

Thank you, i changed the findMax method to return a IntNode and it seems to be working better. However, my sorting method is apparently wrong because it only outputs the 1st and last node. Here are my updated methods and output:

private static IntNode listSort(IntNode head)
	{
		IntNode sorted = new IntNode(head.getData(), null);
		while(head.getLink() != null)//the first list has nodes
		{
			//1. Find the node with the largest element
			if (head.getData() >= sorted.getData())
			{
				//sorted = head;//Add it to the new list
				head.removeNodeAfter();//remove it from the original
				head = head.getLink();
				sorted = new IntNode(head.getData(), sorted);
			}
			else
			{
				System.out.println("Empty.");
			}
		}
		return sorted;	
	}
	
	private static IntNode findMax1(IntNode head)
	{
		//for loop to traverse the linked list
		for (IntNode cursor = head; cursor.getLink()!= null; cursor = cursor.getLink())
		{
			if (cursor.getData() > head.getData())//if the specific element is greater than 0 or a max value already set...
			{
				head = cursor;//It will change the max value to the element
			}
		}
		return head;//Returns the max value as an int to be used to sort the linked list
	} 

Here is my output, i know the numbering is wrong my counter is working properly in my for loops for some reason:

List Length: 4
1: 85
1: 105
1: 98
1: 3
the final max value is: 105
The sorted list.
1: 3
1: 105

[n8wxs]

You are initializing the counter inside the loop. Move it:

...
//display the linked list
int count = 1;
for (cursor = head; cursor != null; cursor = cursor.getLink()) {
    System.out.println(count + ": " + cursor.getData());
    count++;
}
...
//////////// here too
...
sorted = listSort(head);
count = 1;
for (cursor = sorted; cursor != null; cursor = cursor.getLink()) {
    System.out.println(count + ": " + cursor.getData());
    count++;
}
...

I suggest you look into using a bubble sort for your list. No need to remove and add nodes to lists. Simply swap the data values into order.

This post has been edited by n8wxs: 24 October 2010 - 10:27 AM

[billinkw]

Thanks again. I will read up on bubble sorts, not really familiar with them. Thanks again for the feedback.

[macosxnerd101]

n8wxs, on 24 October 2010 - 12:32 PM, said:

I suggest you look into using a bubble sort for your list. No need to remove and add nodes to lists. Simply swap the data values into order.

Sorry, n8wxs, but I have to disagree here. It sounds like the OP is going after an Insertion Sort algorithm to me, which is far more efficient than Bubblesort with standard arrays, and even more so with Linked Lists b/c there is no need to shift elements down in a Linked List. This is done implicitly on the insertion. Because of this, Mergesort and Insertion Sort are the more efficient sorting algorithms when dealing with Linked Lists, which is why Collections.sort() uses Mergesort (b/c it has to handle ArrayLists and LinkedLists) while Arrays.sort() uses Quicksort (as it only has to deal with arrays).

[billinkw]

I looked over some tutorials but they seem to only apply to arrays. Can i get a few pointers for my case. Thanks.

[macosxnerd101]

Which algorithm are you asking about?

[billinkw]

Both. I am not very familiar with linked sets so I am having some difficulty. The tutorials i found for bubble sort and insertion sort were for arrays only.

[macosxnerd101]

Let's work through this given a Linked List with the contents 3 --> 2 --> 5 --> 1 --> 4 .

For Insertion Sort:
So we start with element 1, which is 2. Is 2 < 3? Yes, so 2 is the new Head Node, and head.next points to the Node for 3. With arrays, you would have to shift 3 down one spot manually, and then insert 2 at index 0. Simply modifying the pointers handles this for Linked List. Next we pull out 5. As it is in order, we don't do anything. Next we pull out 1, and make it the new Head Node, with head.next pointing to the Node for 2. Finally we pull out 4. As 3 < 4 < 5, we insert 4 between the Node for 3 and 5, with node3.next pointing to the node4, and node4.next pointing to node5. Basically, this is like a standard add(index, elem) method.

For Bubble Sort:
The big deal is the swap. Basically, if you have two Nodes you need to swap, you pull out the end Node and stick it before the beginning Node modifying the next pointers as I described in Insertion Sort. So to swap 2 and 3, 2.next should point to 3, and 3.next should now point to 5.

[n8wxs]

I suggested the bubble sort as it is the simplest to implement:

private static void listSort(IntNode head) {
    int length = IntNode.listLength(head);

    if (length < 2) {
        return;
    }

    boolean swapped = true;
    int temp;
    IntNode nodeA, nodeB;

    while (swapped) {
        swapped = false;

        nodeA = head;
        nodeB = nodeA.getLink();

        for (int i = 0; i < length - 1; i++) {
            if (nodeA.getData() > nodeB.getData()) {
                temp = nodeB.getData();
                nodeB.setData(nodeA.getData());
                nodeA.setData(temp);
                swapped = true;
            }
            nodeA = nodeB;
            nodeB = nodeA.getLink();
        }
    }
}

Note that no node is removed/inserted. We simply walk the list. A bunch of times.

This post has been edited by n8wxs: 24 October 2010 - 02:33 PM