Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have a server communicating over network interface. This server should run on a background thread, so that it does not block the ui thread. The ui thread starts and stops the server. Even starting and stopping should be done from a background thread.

Currently I wrap all public methods of the server class in a thread that gets started immediately:

class Server {

  public void start() {
    new Thread() {
      public void run() {
        handleStart();
      }
    }.start();
  }

  public void stop() {
    new Thread() {
      public void run() {
        handleStop();
      }
    }.start(); 
  }

  private void handleStart() {
    startLooperThread();
  }

  private void handleStop() {
    stopLooperThread();
  }
}

Is this the proper way to do this or are there any neat alternatives provided by JDK (or any other library)?

share|improve this question
    
This is missing a lot of context for giving really tailored advice, but I'll try anyways.. – Vogel612 Nov 28 '14 at 8:46

Disclaimer: This answer uses the lambda-feature for java 8. Assuming non-availability of java 8, the expressions can be easily converted to Runnables.

You're completely overexerting yourself. Use inbuilt features of Java with a little composition and your server becomes by far easier:

public class Server {
     private final ExecutorService threadPool = Executors.newFixedThreadPool(1);

     public Server() {}

     public void start() {
         threadPool.execute(() -> handleStart()); //Runnable
     }

     public void stop() {
         threadPool.execute(() -> handleStop());
         threadPool.shutdown(); //depends a little on your functionality
     }
     //rest can remain unchanged
}

The Thread-Pool can be expanded manually. Additionally I'd recommend to override finalize() to shut down your executor.

share|improve this answer
    
Yeah. My question was deliberately broad, as I encountered similiar 'problems' often. But your solution looks nice. – matcauthon Nov 28 '14 at 9:11

What you have done is not the ideal way. For UI applications that generate events processed by a "Server", there are established design patterns. One such pattern is the "Event loop" or the "Scheduler queue". Goes something like this (In a pseudo-Java like language that is not quite verbose):

UI:

void onSomeUIEvent(UIEvent ev) {
    ServerContext.pushEvent(ev);
}

ServerContext:

static void pushEvent(UIEvent ev) {
    _blockingQueue.push(ev);
}

Server:

class Server {

    private BlockingQueue<UIEvent> _blockingQueue; //Shared with ServerContext
    Thread worker {
        void run() {
            UIEvent e = _blockingQueue.pop();
            //Now do something with the event, like start/stop something

        }
    }
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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