Help with threads/printout.

[Mike Brooks]

The goal is to start x amount of threads. Save the number of the thread, the start time and the end time, then print it all out at the end. Except for my printout I get all zeros. This is my first attempt dealing with threads. So any help is appreciated.

**Skip the code below and go to my next post for recent code.**

import java.util.*;
 
public class ThreadApp implements Runnable{
 
    protected int root, rootAnswer, arrayPlace = -1;
    
    int[] threadNum = new int[5];                // Thread ID number
    
    long[] threadStart = new long[5];    // Thread start time
    long[] threadEnd = new long[5];      // thread end time
    
    Random rand = new Random();
    
    Date time = new Date();
   
    // Run method
    public void run(){
        
        arrayPlace++;
        
        threadStart[arrayPlace] = time.getTime();
        threadNum[arrayPlace] = arrayPlace + 1;    // Places ID's in array
        
        // Puts thread to sleep
        try {
            Thread.sleep(2000);
        } catch (InterruptedException ex){
            ex.printStackTrace();
        }
 
        root = rand.nextInt(50) + 11;  // Generates a andom num between 10-49
        rootAnswer = root * root;      // Finds the square root of number
        
        threadEnd[arrayPlace] = time.getTime();
        
       
    }
 
    public String toString(){
        
        String threadInfo = "";
        
        for (int i=0; i < 5; i++){
            
            threadInfo = threadInfo + "\nThread#" + threadNum[i] + "\t" +
                    "<" + threadStart[i] + ">" + "\t" + "<" + threadEnd[i] +
                    ">";
        }
 
        return threadInfo;
    }
 
}    

Driver class

  import java.util.*;
 
public class Driver extends ThreadApp{
    
    public static void main (String[] args){
        
        Random rand = new Random();      // Used to generate random numbers.
        ThreadApp list = new ThreadApp();
        Runnable work = new ThreadApp();
        Thread myThread;
        
        int randomInt = rand.nextInt(5) + 1; // Generates random num of thread.
        
        // For loop for however many threads are generated.
        for (int i=0; i < randomInt; i++){
 
            myThread = new Thread(work);
            myThread.start();   // Start the thread.      
           
        }
        
        System.out.println(list);
 
    } 
}
 
  

[ October 10, 2008: Message edited by: Mike Brooks ]

[Ernest Friedman-Hill]

Hi Mike,

So... gotta minute (or a whole bunch of minutes?)

First, the reason you're getting all zeros: in Driver.main(), "list" and "work" refer to two different instances of ThreadApp, each with their own copy of the member variables threadNum, threadStart, threadEnd, etc. The ones you're printing out are stored in "list"; the ones you're actually setting are stored in "work." The "list" ones are never set. If you don't understand this, stop. Don't worry about threads until that sort of basic object-oriented stuff is clear to you.

If that was just a momentary lapse of reason, then we'll move onto non-thread-related issue number two. Objects should hold their own individual data; it's very non-Java-like to use an array in a class to hold a list of values for the instances of a class. Why not have ThreadApp look like this:

public class ThreadApp implements Runnable {
    private int id;
    private long threadStart, threadStop;
    public ThreadApp(int id) { this.id = id; }
    public void run() {
        threadStart = System.currentTimeMillis();
        // Sleep, do work, whatever
        threadEnd = System.currentTimeMillis();
    }
    public String toString() {
        return "\nThread#" + id + "\t" +
                    "<" + threadStart + ">" + "\t" + "<" + threadEnd +
                    ">";
    }
}

For this class, main() would supply a unique id for each of five Runnables; each would hold all its own data. Instead of using toString() to print all five results, I'm letting it print the results for just the one object; main() would need a loop over the five Runnables (by the way, main() would hold the five Runnables in an array.) Make sense? That's the Java way of doing things.

One more non-thread-related things: a Date object represents a specific date and time; by default, the instant it was constructed. Calling getTime() on a Date will always return that same time; it won't update as time passes. Use System.currentTimeMillis() for that.

Now, the thread stuff: threads are complicated. You can not let two threads read and write the same variable (as happens here many times) without using some form of synchronization. This lesson in Sun's Java Tutorial should give you an idea of the issues involved.

Anyway, there's just so much wrong here that there's no quick answer on how to fix it. Slow down a bit, take your time, walk before you run, etc. You'll get there, don't worry!
}

[Mike Brooks]

From reading the lesson and using the book, "Head First Java". The book says just to add synchronized to the method doing the work.

Here is the updated code. It prints out the id fine, but when it goes to print out "work", it just prints out the name of the class with some numbers and letters, instead of its toString(). Any suggestions?

I have my array of Runnables and set the thread name in the main class.


Driver Class

public class Driver extends ThreadApp{
    
    public static void main (String[] args){
        
        Random rand = new Random();      // Used to generate random numbers.
        Thread myThread;
        
        int randomInt = rand.nextInt(5) + 1; // Generates random num of thread.
        
        ThreadApp[] work = new ThreadApp[randomInt];
        for (int i=0; i < work.length; i++)
            work[i] = new ThreadApp();
        
        // For loop for however many threads are generated.
        for (int i=0; i < randomInt; i++){
 
            myThread = new Thread(work[i]);
            myThread.setName(String.valueOf(i+1));
            myThread.start();   // Start the thread.
            
            System.out.println("\nID" + myThread.getName() + work[i]);
            
        }   
 
    } 
}
   

ThreadApp class

public class ThreadApp implements Runnable{
 
    private int root, rootAnswer;
    private long threadStart, threadEnd;
    
    Random rand = new Random();
   
    // Run method
    public void run(){
        
        threadStart = System.currentTimeMillis();
        
        root();
        
        threadEnd = System.currentTimeMillis();           
    }
    
    private synchronized void root(){
        
        // Puts thread to sleep
        try {
           Thread.sleep(2000);
        } catch (InterruptedException ex){
            ex.printStackTrace();
        }
 
        root = rand.nextInt(50) + 11;  // Generates a random num between 10-49
        rootAnswer = root * root;      // Finds the square root of number
    }
 
    public String toString(){
        
        String threadInfo = "";
     
            threadInfo = "<" + threadStart + ">" + 
                    "\t" + "<" + threadEnd + ">";
 
        return threadInfo;
    }
 
}
 
    

[ October 10, 2008: Message edited by: Mike Brooks ]

[ October 10, 2008: Message edited by: Mike Brooks ]

[ October 10, 2008: Message edited by: Mike Brooks ]
[ October 10, 2008: Message edited by: Mike Brooks ]

[Steve Luke]

You aren't printing out the ThreadApp objects, you are printing the work object which is an array. To get to the ThreadApp objects' toString method you need to loop through the array and print the individual values.

[Steve Luke]

Also:

myThread = new Thread(work[i]);

Does this code work? In your code, work[i] would be null, you never initialize it. If that line doesn't throw a null pointer exception (it may not...) then it still wouldn't be able to execute the ThreadApp because the ThreadApp doesn't exist.

Also:

rootAnswer = root * root;      // Finds the square root of number

That isn't a calculation for square root. It is the square of a variable called root. Much different.

[Mike Brooks]

What do you mean by not initialized, when I have

  ThreadApp[] work = new ThreadApp[randomInt];  

I tried using...

System.out.println("\nID" + myThread.getName() + work[i]);    

instead of what I have. But like you said in your second post, it just gives null. Neither worked, so I just posted what I had.

[Ernest Friedman-Hill]

This line

ThreadApp[] work = new ThreadApp[randomInt];

creates an array of variables each of which could point to a ThreadApp object but which is currently null. To fill it, you need a for-loop which assigns to each element of the array; i.e.,

ThreadApp[] work = new ThreadApp[randomInt];  
for (int i=0; i<work.length; ++i)
    work[i] = new ThreadApp(i);

[Mike Brooks]

Alright, I got that. But now I just get 0's for start time and end time. Sometimes it might print start date for x amount of threads. If it does print start dates for multiple threads, its the same time. ?

I did some stepping through the code and noticed when it got to myThread.start(), it would not go to the ThreadApp class run(). After myThread.start() is goes straight to System.out.println("\nID" + myThread.getName() + work[i]); and of course I get zero's. Any suggestions on why the thread is not running?

 public class Driver extends ThreadApp{
    
    public static void main (String[] args){
        
        Random rand = new Random();      // Used to generate random numbers.
        Thread myThread;
        
        int randomInt = rand.nextInt(5) + 1; // Generates random num of thread.
        
        ThreadApp[] work = new ThreadApp[randomInt];
        for (int i=0; i < work.length; i++)
            work[i] = new ThreadApp();
        
        // For loop for however many threads are generated.
        for (int i=0; i < randomInt; i++){
 
            myThread = new Thread(work[i]);
            myThread.setName(String.valueOf(i+1));
            myThread.start();   // Start the thread.
            
            System.out.println("\nID" + myThread.getName() + work[i]);
            
        }   
        
    } 
}   

[ October 10, 2008: Message edited by: Mike Brooks ]
[ October 10, 2008: Message edited by: Mike Brooks ]

[Steve Luke]

You aren't waiting for the threads you start to finish. You are starting them all at the same time, then immediately printing out the results, which may happen before the startTime gets set and definitely will happen before any endTime gets set.

You need to join() your work threads before printing their results so that you know they are completed.

[Mike Brooks]

Maybe im putting the join() in the wrong place, but when I place it like below. The next thread doesn't start until the previous thread ends.(indicated by the finish time of the previous thread's end time and the next thread's start time). I want them to all start as soon as possible/concurrently. Any suggestions?

 public class Driver extends ThreadApp{
    
    public static void main (String[] args){
        
        Random rand = new Random();      // Used to generate random numbers.
        Thread myThread;
        
        int randomInt = rand.nextInt(5) + 1; // Generates random num of thread.
        
        Runnable[] work = new ThreadApp[randomInt];
        for (int i=0; i < work.length; i++)
            work[i] = new ThreadApp();
        
        // For loop for however many threads are generated.
        for (int i=0; i < randomInt; i++){
 
            myThread = new Thread(work[i]);
            myThread.setName(String.valueOf(i+1));
            myThread.start();   // Start the thread.
            
            // Waits for thread to die before printing out.
            try {
                myThread.join();
            } catch (InterruptedException ex){
                ex.printStackTrace();
            }
            
            System.out.println("\nID" + myThread.getName() + work[i] + "  " +
                    "Thread terminated");
            
        }   
 
    } 
}   

[ October 11, 2008: Message edited by: Mike Brooks ]

[Steve Luke]

That is right, you aren't putting the join in the right place. How do you think you could put it in the right place? Where do you think the join's should actually go, knowing that the current placement is wrong.

[Mike Brooks]

myThread.join() has to be in the for loop and putting it before start() just gives zeros for start time and end time. So theres no other place to really put it, unless there is something else or other changes that need to be made.

All examples I have seen are something like

thread1.start();
thread2.start();

try {
thread1.join.();
thread2.join();
} catch (InterruptedException ex){
ex.printStackTrace();
}

So all examples are calling start before joining them. So is the fix to start all of them before joining them? I can start them all, but I don't see a way to call join() on all of them same set of myThreads. Any suggestions?

[Steve Luke]

Originally posted by Mike Brooks:
myThread.join() has to be in the for loop ...
So is the fix to start all of them before joining them? I can start them all, but I don't see a way to call join() on all of them same set of myThreads. Any suggestions?

Yes, you have to start them all, then join() them after all are started. You would need to store references to the Threads in an array so you can make them, start them, and store them in one for loop, then join them in a second.