4
\$\begingroup\$

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.
\$\endgroup\$
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ – TheCoffeeCup Feb 21 '15 at 17:53
1
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ I tried to make it as readable as possible. \$\endgroup\$ – Koray Tugay Feb 21 '15 at 17:58
  • \$\begingroup\$ Oh you succeeded, it's definitely nicely readable! (It will still be so after applying my suggestions ;-) \$\endgroup\$ – Stop ongoing harm to Monica Feb 21 '15 at 17:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.