Currently, the following code runs and produces the expected and desired output. However, I'm sure that there's much wrong with it.
This project queries the (flat) database, instantiates each row as an object, adds those objects to a queue, and makes the queue available to clients. Clients may then connect to the server, which pops objects from the queue and delivers that single object to the client. The client receives the object, the user updates it, and then it is sent back to the server. The server then updates the database accordingly.
So far as I can tell, this is functioning correctly, and allows for multiple clients.
However, I'm quite sure that the control flow is "odd", or at least sub-optimal.
How can I make the code clearer by using, for example, producer/client or another pattern or idiom?
WorkerRunnable:
package net.bounceme.dur.server.streams;
import net.bounceme.dur.jdbc.Title;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.net.Socket;
import java.util.NoSuchElementException;
import java.util.logging.Level;
import java.util.logging.Logger;
import net.bounceme.dur.jdbc.Queries;
public class WorkerRunnable implements Runnable {
private static final Logger log = Logger.getLogger(WorkerRunnable.class.getName());
protected Socket socket = null;
private RecordsQueueWrapper recordsQueue = null;
private final Queries queries = new Queries();
public WorkerRunnable(Socket clientSocket, RecordsQueueWrapper recordsQueue) {
this.socket = clientSocket;
this.recordsQueue = recordsQueue;
}
@Override
public void run() {
Title serverTitle = null;
Title clientTitle = null;
boolean queueEmpty = false;
try (ObjectOutputStream objectOutputStream = new ObjectOutputStream(socket.getOutputStream());
ObjectInputStream objectInputStream = new ObjectInputStream(socket.getInputStream())) {
do {
try {
serverTitle = recordsQueue.pop();
log.info(serverTitle.toString());
objectOutputStream.writeObject(serverTitle);
log.fine(serverTitle.getClass().getCanonicalName());
clientTitle = (Title) objectInputStream.readObject();
queries.updateTitle(clientTitle);
} catch (NoSuchElementException nse) {
queueEmpty = true;
}
} while (!queueEmpty);
} catch (java.io.EOFException eofe) {
log.log(Level.WARNING, "why is there no record from client?\n", eofe);
} catch (IOException | ClassNotFoundException ex) {
Logger.getLogger(WorkerRunnable.class.getName()).log(Level.SEVERE, null, ex);
}
}
}
Server:
package net.bounceme.dur.server.streams;
import net.bounceme.dur.driver.MyProps;
import java.net.ServerSocket;
import java.net.Socket;
import java.io.IOException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.logging.Logger;
public class Server implements Runnable {
private static final Logger log = Logger.getLogger(Server.class.getName());
protected int serverPort = 8080;
protected ServerSocket serverSocket = null;
protected boolean isStopped = false;
protected Thread runningThread = null;
protected ExecutorService threadPool = Executors.newFixedThreadPool(10);
private final RecordsQueueWrapper instance = RecordsQueueWrapper.getInstance();
public Server(MyProps properties) {
serverPort = properties.getServerPort();
}
@Override
public void run() {
synchronized (this) {
this.runningThread = Thread.currentThread();
}
openServerSocket();
while (!isStopped()) {
Socket clientSocket = null;
try {
clientSocket = this.serverSocket.accept();
} catch (IOException e) {
if (isStopped()) {
log.warning("stopped server");
return;
}
throw new RuntimeException("Error accepting client connection", e);
}
this.threadPool.execute(new WorkerRunnable(clientSocket, instance));
}
this.threadPool.shutdown();
log.info("server stopped");
}
private synchronized boolean isStopped() {
return this.isStopped;
}
public synchronized void stop() {
this.isStopped = true;
try {
this.serverSocket.close();
} catch (IOException e) {
throw new RuntimeException("Error closing server", e);
}
}
private void openServerSocket() {
try {
this.serverSocket = new ServerSocket(this.serverPort);
} catch (IOException e) {
throw new RuntimeException("Cannot open port 8080", e);
}
}
}
I'm just looking for the single biggest, glaring opportunity of what to fix with this structure and flow; pseudo-code is more than sufficient.
