Multi-Threaded Java Server

Question

As practice I wanted to write a Socket in Java:

/* User: koray@tugay.biz 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: koray@tugay.biz 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.

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...