In his Clean Code book, Robert C. Martin states that functions should do only one thing and one should not mix different levels of abstraction. So I started wondering whether the code below meets these requirements (I assume the answer is negative). Which one of the following code snippets is better?
Snippet 1
public void run() {
isRunning = true;
try {
serverSocket = new ServerSocket(port);
}
catch (IOException | IllegalArgumentException e) {
System.out.println("Could not bind socket to given port number.");
System.out.println(e);
System.exit(1);
}
while(isRunning) {
Socket clientSocket = null;
try {
clientSocket = serverSocket.accept();
}
catch (IOException e) {
logEvent("Could not accept incoming connection.");
}
Client client = new Client(clientSocket, this);
connectedClients.put(client.getID(), client);
Thread clientThread = new Thread(client);
clientThread.start();
logEvent("Accepted new connection from " + clientSocket.getRemoteSocketAddress().toString());
client.send("@SERVER:HELLO");
}
}
Snippet 2
public void run() {
isRunning = true;
createServerSocket();
while(isRunning) {
Socket clientSocket = null;
clientSocket = acceptConnection();
addClient(clientSocket);
}
}
private void createServerSocket() {
try {
serverSocket = new ServerSocket(port);
}
catch (IOException | IllegalArgumentException e) {
System.out.println("Could not bind socket to given port number.");
System.out.println(e);
System.exit(1);
}
}
private Socket acceptConnection() {
try {
Socket clientSocket = serverSocket.accept();
}
catch (IOException e) {
logEvent("Could not accept incoming connection.");
}
return clientSocket;
}
private void addClient(Socket clientSocket) {
Client client = new Client(clientSocket, this);
connectedClients.put(client.getID(), client);
Thread clientThread = new Thread(client);
clientThread.start();
logEvent("Accepted new connection from " + clientSocket.getRemoteSocketAddress().toString());
client.send("@SERVER:HELLO");
}
However, splitting such simple code into multiple fragments seems like an overkill (although it's more readable - is it?). Besides, createServerSockets
has side effects (Sys.exit
) and addClient
does too many things at once. Any advice?