Skip to content

Instantly share code, notes, and snippets.

@ceharris
Last active December 16, 2015 22:39
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save ceharris/5508524 to your computer and use it in GitHub Desktop.
Save ceharris/5508524 to your computer and use it in GitHub Desktop.
class AbstractSocketAppender extends ...
implements EventDispatcher, ExceptionHandler {
private SocketConnectorFactory connectorFactory;
private ConnectionRunner connectionRunner;
private Future<?> runnerTask;
public void start() {
...
// extract method to create connector factory...
connectorFactory = new DefaultSocketConnectorFactory();
connectorFactory.setInetAddress(address);
connectorFactory.setPort(port);
connectorFactory.setDelayStrategy(...);
connectorFactory.setExceptionHandler(this);
// extract method to create connection runner...
connectionRunner = new ConnectionRunner(connectorFactroy, this);
connectionRunner.setContext(getContext());
runnerTask = getContext().getExecutorService().submit(connectionRunner);
...
}
public void dispatchEvents(Socket socket) throws InterruptedException {
...
}
}
class ConnectionRunner extends ContextAwareBase implements Runnable {
private final SocketConnectorFactory connectorFactory;
private final EventDispatcher eventDispatcher;
public final void run() {
try {
while (!Thread.currentThread().isInterrupted()) {
SocketConnector connector = connectorFactory.createConnector();
Future<Socket> socketFuture = getContext().getExecutorService().submit(connector);
Socket socket = socketFuture.get();
eventDispatcher.dispatchEvents(socket);
}
} catch (InterruptedException ex) {
assert true; // ok... we'll exit now
} catch (ExecutionException ex) {
addError("connector error: " + ex);
}
addInfo("shutting down");
}
}
@ceharris
Copy link
Author

ceharris commented May 3, 2013

Might even be reasonable to refer to this as a ClientRunner to parallel ServerRunner.

@ceki
Copy link

ceki commented May 3, 2013

As I am less familiar with ServerRunner, this feels outside of my comfort zone. (That's my problem not necessarily a problem with the code). I have a closer look at the server-side of things and come back with an informed opinion.

@ceki
Copy link

ceki commented May 3, 2013

ConnectionRunner would need to know about the dispatcher (SocketAppender or SocketAppender) whereas with the approach outlined in [1], the ConnectionRunner only knows about retrieving connections (Sockets) and not much else.

[1] https://gist.github.com/ceki/5508403

@ceharris
Copy link
Author

ceharris commented May 3, 2013

True, but [1] does not eliminate as much duplicate code. I see ConnectionRunner's role as coordinating the process of joining a connected socket and an event dispatcher, while carefully observing thread interruption. Essentially, this is a rather textbook use of the Template Method design pattern. ConnectionRunner.run is a template; the behavior that varies is represented by the EventDispatcher. By declaring the EventDispatcher as an interface, we can easily and completely test all paths in ConnectionRunner.run() by mocking the EventDispatcher and SocketConnectorFactory interfaces, without needing a real Socket object and with no asynchrony in the test.

@ceharris
Copy link
Author

ceharris commented May 3, 2013

One of the deficiencies in the existing tests of AbstractSocketAppender and SocketReceiver tests is the design prevents path testing in the run method. The tests use socket I/O to validate that events are being dispatched properly, but that's more of an integration test than a unit test, and is more fragile across platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment