patternjavaMinor
Client Sends XML, Server Parses XML, Queues Up and Executes Commands
Viewed 0 times
executescommandsandxmlclientparsesserverqueuessends
Problem
I recently had a Java interview where I was asked to write a client-server application. I don't code primarily in Java, but with my preliminary Java knowledge, I thought I built a reasonable working solution. After evaluating my solution, I was told that I wasn't going to be interviewed further. I'm fine with their decision, but I'd like to know what parts of my solution weren't right. Any feedback on my code below will be greatly appreciated.
These were the requirements:
Client: Program called ClientEmulator
-
Command line option to specify TCP/IP Address, port, and file to read XML data file.
-
Should open XML data file and send to server. XML will be of this format
-
Display the response sent by server to the console.
Server: Program called ServerSocketMonitor
-
Command line option to specify TCP/IP Address and port
-
Socket Listener: Open socket and listen for incoming data
-
Send client a receipt notice when a command is received
-
Application must remain operational until CTL+C is detected
-
Server will create a command queue and process each client command in a separate thread
-
Server's execution/processing of these commands was supposed to be dumb: just print the command name and the execution time
This was my solution:
Client
```
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.Socket;
/**
*
* @author DigitalEye
*/
public class ClientEmulator {
// Singleton Implementation: In the context of this app,
private static ClientEmulator _singletonInstance = null;
// Default constructor is private to make sure it can't be accessed outside
// the class
private ClientEmulator() {
}
/**
* Returns single instance of ClientEmulator
* @return Instance of ClientEmulator
*/
public static ClientEmulator getSingleClientEmulator() {
These were the requirements:
Client: Program called ClientEmulator
-
Command line option to specify TCP/IP Address, port, and file to read XML data file.
-
Should open XML data file and send to server. XML will be of this format
Print
Display
Query
-
Display the response sent by server to the console.
Server: Program called ServerSocketMonitor
-
Command line option to specify TCP/IP Address and port
-
Socket Listener: Open socket and listen for incoming data
-
Send client a receipt notice when a command is received
-
Application must remain operational until CTL+C is detected
-
Server will create a command queue and process each client command in a separate thread
-
Server's execution/processing of these commands was supposed to be dumb: just print the command name and the execution time
This was my solution:
Client
```
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.Socket;
/**
*
* @author DigitalEye
*/
public class ClientEmulator {
// Singleton Implementation: In the context of this app,
private static ClientEmulator _singletonInstance = null;
// Default constructor is private to make sure it can't be accessed outside
// the class
private ClientEmulator() {
}
/**
* Returns single instance of ClientEmulator
* @return Instance of ClientEmulator
*/
public static ClientEmulator getSingleClientEmulator() {
Solution
Two of the specific requirements on the server side are:
These two requirements indicate that they want a 'standard' server system, and probably a standard Executor service.
I would expect to see code that looks something like:
The above code will listen for multiple clients, not just one.
It will handle each client in a separate thread.
Update: The question: "My solution was to create a separate command processor thread, which would then span a new thread for each command. Any comments on my approach to ensure that commands are processed in the order that they are received?"
There are two parts to processing the commands... the first part is that they should be acknowledged to the client, and it is appropriate that the ack is in the correct order. There is no requirement that the commands are actually processed in that same order.
Notes:
The network server I described above is pretty typical. Here are some results from google which describe it:
The point is that they all follow the same pattern, and it's a good one.
Now, your problem is that a client may send more than one request. This is a problem because.... you need a protocol to support it.
One of the big 'gotcha's' with network processing is that data does not arrive when you expect it. For example, in your client, it may just hang forever.... because you never close the OutputStream in the client..... in fact.... Your server WILL NOT complete the parsing of the client's data.... --- I just realized that you force all the XML on to one line, and that my previous comment is wrong... you need to **document these things.
I had previously assumed that the protocol was a simple:
But, as 200_success points out, it is nothing of the sort.... you should be forcing the data through the socket, and you are not! You are not flushing the output stream, and you are not closing it.
As a result the timing of when your client request is actually sent is unpredictable.
So, your client-side management is poor.... for a simple single-method client, you have managed to put a lot of broken things in there. I would recommend the following main-method client:
Given all the above, I would recommend that the socket-server management/configuration is not changed from my previous recommendation, but each client-socket-handling thread in the server should look something like:
```
class MyClientHandler implements Runnable {
// have one thread pool running with as many threads as there are CPU's
// this thread pool will process the actual server commands.
private static final ExecutorService COMMANDSERVICE = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
private final Socket clientSocket;
MyClientHandler(Socket client) {
clientSocket = client;
}
public void run() {
// parse the XML from the c
- server must run until Ctrl-c is pressed.
- it must process commends in a queue in a separate thread.
These two requirements indicate that they want a 'standard' server system, and probably a standard Executor service.
I would expect to see code that looks something like:
ServerSocket serversocket = new ServerSocket();
serversocket.bind(....);
ExecutorService threadpool = new Executors.newCachedThreadPool();
while (true) {
final Socket client = serversocket.accept();
// will need to create this class that handles the client.
// the class should implement Runnable.
Runnable clienthandler = new MyClientHandler(client);
threadpool.submit(clienthandler);
}The above code will listen for multiple clients, not just one.
It will handle each client in a separate thread.
Update: The question: "My solution was to create a separate command processor thread, which would then span a new thread for each command. Any comments on my approach to ensure that commands are processed in the order that they are received?"
There are two parts to processing the commands... the first part is that they should be acknowledged to the client, and it is appropriate that the ack is in the correct order. There is no requirement that the commands are actually processed in that same order.
Notes:
- *I only just saw that the XML input file was in the question, but 'hidden'... I edited the Q and made it show up properly)
- I never reviewed this part of your problem the first time around.... it is 'bigger' than just the server-socket thread.
- the more I look in to it, the more problems I am seeing..... this is frustrating...
- your server, as it stands, does not notify the client when it receives each command, it just notifies when it receives the first data... and does not print each command....
The network server I described above is pretty typical. Here are some results from google which describe it:
- The Oracle tutorial
- another Oracle tutorial
- A Pattern/Framework for Client/Server Programming in Java
- A multi-threaded socket-based server
The point is that they all follow the same pattern, and it's a good one.
Now, your problem is that a client may send more than one request. This is a problem because.... you need a protocol to support it.
- the protocol I imagined in a simple case was that the client 'dumps' the request, and the server processes it, and responds.
- if the client can send multiple requests... how do we know when one request ends, and the next one starts?
- XML input cannot just be read from the socket... because any additional client requests will cause the XML to become invalid....
- your current code just reads a file from disk, and dumps it to the socket.... and does not 'parse' the input in to individual requests. Actually, your code strips all the newlines from the input XML before sending it, so the XML is only one one line.... you should comment that, because it is not obvious.
One of the big 'gotcha's' with network processing is that data does not arrive when you expect it. For example, in your client, it may just hang forever.... because you never close the OutputStream in the client..... in fact.... Your server WILL NOT complete the parsing of the client's data.... --- I just realized that you force all the XML on to one line, and that my previous comment is wrong... you need to **document these things.
I had previously assumed that the protocol was a simple:
- client opens socket
- dumps request on socket's OutputStream
- flushes stream
- closes stream
- listens on socket's InputStream for server response.
- processes responses until InputStream is closed
- done.
But, as 200_success points out, it is nothing of the sort.... you should be forcing the data through the socket, and you are not! You are not flushing the output stream, and you are not closing it.
As a result the timing of when your client request is actually sent is unpredictable.
So, your client-side management is poor.... for a simple single-method client, you have managed to put a lot of broken things in there. I would recommend the following main-method client:
- nevermind, use 200_successes recommendation. It is good.
Given all the above, I would recommend that the socket-server management/configuration is not changed from my previous recommendation, but each client-socket-handling thread in the server should look something like:
```
class MyClientHandler implements Runnable {
// have one thread pool running with as many threads as there are CPU's
// this thread pool will process the actual server commands.
private static final ExecutorService COMMANDSERVICE = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
private final Socket clientSocket;
MyClientHandler(Socket client) {
clientSocket = client;
}
public void run() {
// parse the XML from the c
Code Snippets
ServerSocket serversocket = new ServerSocket();
serversocket.bind(....);
ExecutorService threadpool = new Executors.newCachedThreadPool();
while (true) {
final Socket client = serversocket.accept();
// will need to create this class that handles the client.
// the class should implement Runnable.
Runnable clienthandler = new MyClientHandler(client);
threadpool.submit(clienthandler);
}class MyClientHandler implements Runnable {
// have one thread pool running with as many threads as there are CPU's
// this thread pool will process the actual server commands.
private static final ExecutorService COMMANDSERVICE = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
private final Socket clientSocket;
MyClientHandler(Socket client) {
clientSocket = client;
}
public void run() {
// parse the XML from the client...
try (Writer writer = new OutputStreamWriter(clientSocket.getOutputStream())) {
Document document = parseXMLDocument(clientSocket.getInputStream());
NodeList messageNodes = xmlDoc.getElementsByTagName("Command");
for (int i = 0; i < messageNodes.getLength(); i++) {
final String command = ((Element)messageNodes.item(i)).getTextContent();
writer.write(String.format("Processing command %d - %s\n", i + 1, command));
writer.flush(); // send the data to the client....
COMMANDSERVICE.submit(new Runnable() {
public void run () {
// implemented somewhere else.
// will print date, and command.
processServerCommand(command);
}
});
}
}
}Context
StackExchange Code Review Q#41445, answer score: 8
Revisions (0)
No revisions yet.