Multi-Threaded Java Server

Question

As practice I wanted to write a Socket in Java:

/* User: [email protected] Date: 21/02/15 Time: 13:30 */
import java.io.IOException;
import java.net.ServerSocket;
public class MyServer {
 public static void main(String[] args) throws IOException {
 ServerSocket serverSocket = new ServerSocket(8888);
 while (true) {
 new Thread(new ServerSocketThread(serverSocket.accept())).start();
 }
 }
}

and the rest of it:

/* User: [email protected] Date: 21/02/15 Time: 18:14 */
import java.io.*;
import java.net.Socket;
import java.util.Scanner;
public class ServerSocketThread implements Runnable {
 Socket socket;
 public ServerSocketThread(Socket accept) {
 this.socket = accept;
 }
 @Override
 public void run() {
 try {
 Scanner s = new Scanner(socket.getInputStream());
 String readLine;
 while (!(readLine = s.nextLine()).equals("bye")) {
 System.out.println(readLine);
 }
 new PrintWriter(socket.getOutputStream()).write("Bye then..");
 socket.close();
 } catch (IOException e) {
 e.printStackTrace();
 }
 }
}

I wanted to write it as clean as possible. Any improvements, suggestions?

I can use it like this:

Korays-MacBook-Pro:~ koraytugay$ telnet localhost 8888
Trying ::1...
Connected to localhost.
Escape character is '^]'.
koray
tugay
asdfako
bye
Connection closed by foreign host.

• TheCoffeeCup: Your title should only state what your code does, and not what concerns you have. Also, please give a brief explanation in what your code exactly does.

Answer 1

There's not a lot to say about such a simple program, mainly nitpicks:

• Member fields that you don't need to expose to the outside should be private, such as the socket in ServerSocketThread.

• Whenever you can, make member fields final, for example the socket in ServerSocketThread.

• The Socket parameter in the constructor of ServerSocketThread is poorly named accept. It would be better to call it what it is: socket

• s for a Scanner is not a great name either. How about scanner ?

• It's a bit odd that the server doesn't talk back to the client, just prints stuff on its console.

• If the program gets more complex later:

  • It might be good to move the literal string like "bye" to a constant variable, as it's somewhat special, being the special terminating sequence
  • Printing stack trace on the console is considered bad practice
  • Printing pretty much anything to the console is considered bad practice

  However, at this point, your program being a toy, it doesn't really matter, just for the record...

• Koray Tugay: I tried to make it as readable as possible.
• janos: Oh you succeeded, it's definitely nicely readable! (It will still be so after applying my suggestions ;-)