views:

124

answers:

4

Hello StackOverflow.

I'm working on a java project with my team at work. To summarize, we have a main class that has a method that instantiates and calls a "Save" class. This "Save" class saves files back to a server with a couple of constructors and a handful of visible and non-visible methods. The class is CPU-intensive and time-consuming which prevents the main application from displaying a progress bar dialog window letting the user know the status of the save. They asked me to modify the "Save" class in a way that it will spawn off it's own thread so the rest of the main application can do the smaller tasks of displaying information to the user.

Here is a general concept of it:

class MainApp{  
    ...

    private void doSave()
    {
        Save s = new Save();
        StatusWindow sw = new StatusWindow();

        if save_this
          s.saveThis(sw);
        if save_that
          s.saveThat(sw);

        ...
    }
    ...
}

class Save{
   ...

   public void saveThis(StatusWindow s)
   {
       //alot of code
       s.update;

   }
   public void saveThat(StatusWindow s)
   {
       //alot of code
       s.update;
   }
   ... // some non-visible methods, even more code
 }

I'm am currently a novice with threads in Java, but I have a basic understanding of how they work. From what I understand, a class that implements Runnable, when it is instantiated as a new thread, the run() method is executed. The problem is, since there are different methods for different types of saves for different types of files, how do I implement these methods into the run() method? Is the run() method the only method that gets executed when the class is instantiated in a new thread and .start() is called on it?

What would be a good solution to this issue? Would the "Save" class need to be redesigned to have it implement Runnable?

If more details are needed, please let me know. Thanks for any insight!

Update: Thanks everyone for the help! These solutions will come in handy for the future.

+4  A: 

The simplest way is to make a runnable for each. Instead of parameters being passed into run, make them instance fields.

class SaveThatCommand implements Runnable {
     private final StatusWindow s;
     //constructor that initializes s
     public void run() {
        //save that code
        s.update();
     }
}

An easier way to accomplish this, depending on your requirements, is to make an anonymous inner class

public void doSave(final StatusWindow s) {
    if (saveThis) {
        Thread t = new Thread( new Runnable() {
            public void run() {
               saveThis(s);
            }
        });
        t.start();
    }
    //...
}

And you're slightly incorrect: the run method is executed when it is passed into the constructor of a Thread and then start() is called on that thread.

Mark Peters
Yes I totally forgot to add .start() in there, I will edit my question. Thanx :)
A.Donahue
I like the second suggestion, it seems quick and easy to implement for now
A.Donahue
A: 

There are two ways of doing that:

a) You can move the code with the if-block into the run() method.

b) You can have one class per document type that implements runnable.

Approach a) is simpler because it requires less changes to the existing code. But approach b) is the object oriented way of doing it: "One class per task".

nhnb
+2  A: 

A full solution would be to extend the Runnable class and pass in the parameters required and the type of save necessary to the constructor. Then you can run them with:

new Thread(saveRunnable).start();

A simpler solution would be to implement a pattern like this inside the save class:

public void saveThis(StatusWindow s) {
  Runnable r = new Runnable() {
     private StatusWindow s;
     public Runnable setStatusWindow(StatusWindow s) {
       this.s = s;
       return this;
     }

     @Override
     public void run() {
       this.Save.saveThisInternal(this.s);
     }
  }.setStatusWindow(s);
  new Thread(r).start();
}

public void saveThisInternal(StatusWindow s) {
  //alot of code
  s.update();
}
Erick Robertson
+2  A: 

Your coworkers probably call Save from more than one place in the main application and would like to avoid having to change all the other code to support saving as a parallel operation. Also, in general, most people don't prefer making their own Threads and instead prefer using an ExecutorService. So here is how to do it with only modifying the Save class and using an executor:

class Save{
   private static final ExecutorService executor = Executors.newCachedThreadPoolExecutor();
   //or fixed, or whatever you want. Maybe single thread executor is best if the Save code is not well suited to concurrency.

   static {
       Runtime.getRuntime().addShutdownHook(
           new Thread() {
               public void run() {
                   executor.shutdown();
               }
           }
       );
   }

   public void saveThis(StatusWindow s)
   {
      executor.execute(new SaveThis(s));
   }
   public void saveThat(StatusWindow s)
   {
      executor.execute(new SaveThat(s));
   }
   ... // some non-visible methods, even more code

   private class SaveThis implements Runnable {
       //StatusWindow member variable and constructor
       public void run() {
           //alot of code
           s.update;
       }
   }

   private class SaveThat implements Runnable {
       //StatusWindow member variable and constructor
       public void run() {
           //alot of code
           s.update;
       }
   }
 }
Tim Bender
+1 Thanks Tim! I will keep this mind for the future
A.Donahue