1
\$\begingroup\$

Here is a design to achieve a simple model in Java which uses a Thread and has these features:

  1. Starting the thread with an initial value
  2. Pausing the thread
  3. Resuming the thread with new value
  4. Stopping the thread

Restrictions: Do not use already implemented java libraries (from the concurrent package).

A similar question was asked on SO where I have answered with this code, but I want to get some review comments for the same.

I defined an interface for the model:

/**
 * 
 * @author krishna.k
 * 
 * This defines the methods for the model.
 *
 */
public interface IResumable {
  /**
   * starts the model
   */
  public void requestStart();
  /**
   * requests the model to pause
   */
  public void requestPause();
  /**
   * requests the model to resume with new parameter
   * @param newParam
   */
  public void resumeWithNewParam(int newParam);
  /**
   * terminate the model
   */
  public void requestStop();
}

The concrete Model:

public class ResumableModel implements IResumable {
  private Thread worker;
  private WorkerRunnable work;

  public ResumableModel(int initialValue) {
        work = new WorkerRunnable(initialValue);
        worker = new Thread(work);
  }

  @Override
  public void requestStart() {
        worker.start();
  }

  @Override
  public void requestPause() {
        work.setPauseRequested(true);
  }

  @Override
  public void resumeWithNewParam(int newParam) {
        work.setNewParam(newParam);
  }

  @Override
  public void requestStop() {
        worker.interrupt();
  }

  private static class WorkerRunnable implements Runnable {
        private int param; // we can have the variable of the type depending
                                      // upon
        // the requirement.
        private final Object lock = new Object();
        private volatile boolean isPauseRequested = false;

        public void run() {
              synchronized (lock) {
                    try {
                          while (!Thread.currentThread().isInterrupted()) {
                                while (isPauseRequested) {
                                      lock.wait();
                                }
                                System.out.println("value of param is" + param);
                          }
                    } catch (InterruptedException e) {
                          e.printStackTrace();
                    }
              }
        }

        public WorkerRunnable(int param) {
              this.param = param;
        }

        private void setPauseRequested(boolean isPauseRequested) {
              this.isPauseRequested = isPauseRequested;
        }

        private void setNewParam(int param) {
              // double locking to prevent the calling thread from being locked
              // (if in running state without pause requested then calling thread
              // will be in indefinite wait state for acquiring the lock.
              if (isPauseRequested) {
                    synchronized (lock) {
                          if (isPauseRequested) {
                                this.param = param;
                                this.isPauseRequested = false;
                                lock.notifyAll();
                          } else {
                                // logger will be used in real application
                                System.out.println("Need to pause first before setting a new param"); 
                          }
                    }
              } else {
                    // logger will be used in real application
                    System.out.println("Need to pause first before setting a new param"); 
              }
        }
  }
}

The testing driver code:

public class Test {
  public static void main(String[] args) throws InterruptedException {
        IResumable resumable = new ResumableModel(10);
        resumable.requestStart();

        //Current thread is Main thread
        Thread.currentThread().sleep(3000); // making the main thread sleep for 3 seconds.

        resumable.requestPause();

        Thread.currentThread().sleep(3000); // again making the main thread sleep for 3 seconds.

        resumable.resumeWithNewParam(20);

        Thread.currentThread().sleep(3000); // again making the main thread sleep for 3 seconds.
        resumable.requestStop();
  }
}

Please provide review comments and feedback. It will help me and the people to whom I tell this approach.

\$\endgroup\$
1
\$\begingroup\$
  1. You have a stateful model that has three states: Pausing, Running, Stopped. I would represent this with a State-pattern. For me there is too much micro-handling (isPauseRequested, !Thread.currentThread().isInterrupted()) as the concept of the model is the handling of these states.

  2. As a matter of symmetry I avoid using volatile variables. I currently heard about the concept of "single source of truth" (SSOT) as you have TWO sources that have dependencies: the lock object and the volatile variable. Even for the sake of efficiency I would use the lock object to express the "monitor" for all elements that should be handled in the monitor.

  3. (Optional as you want to follow my path) Even if some literature says something else (I know about that) I prefer synchronized methods. That has for me following reasons: First I want the developer using my objects to know about synchronization BESIDE potential documentation. Second I do not explicitly want to allow other developers using MY objects in THEIR synchronization purposes as I see this as semantically wrong. THEIR implementation should fail (mostly dead locks) because MY implementation is responsible for MY objects and using developers should not take over this responsibility.

  4. As your model is composing the elements to provide the expected functionality I would locate any synchronization mechanisms there as well. Your ResumableModel becomes the monitor. You the either make all model-method synchronized or move the lock-object to the ResumableModel. This depends on if you follow my path or not.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.