1
\$\begingroup\$

This question presents two rather small Java programs: the first one is a simple client that reads text from command line and sends it to the server (TextSender.java); the server does nothing more but echoes the received text on the command line (TextReceiver.java)

TextSender.java:

import java.io.IOException;
import java.io.PrintWriter;
import java.net.Socket;
import java.net.UnknownHostException;
import java.util.Scanner;
public class TextSender {
 public static final int PORT = 1857;
 public static void main(String[] args) {
 if (args.length == 0) {
 System.out.println("Usage: java Main IP");
 return;
 }
 try (Socket clientSocket = new Socket(args[0], PORT);
 PrintWriter out = new PrintWriter(clientSocket.getOutputStream(), true);) {
 Scanner scanner = new Scanner(System.in);
 while (scanner.hasNextLine()) {
 String line = scanner.nextLine();
 if (line.trim().equals("quit")) {
 break;
 }
 // Terminate the line so that the receiver process knows that 
 // the end of input is occured.
 out.print(line + "\n");
 out.flush();
 }
 } catch (UnknownHostException ex) {
 System.err.println("[ERROR] Could not reach host \"" + 
 args[0] + "\".");
 System.exit(1);
 } catch (IOException ex) {
 System.err.println("[ERROR] IO failed on host \"" + 
 args[0] + "\".");
 System.exit(1);
 }
 }
}

TextReceiver.java:

import java.io.BufferedReader;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.Scanner;
import java.util.logging.Logger;
public class TextReceiver implements Runnable {
 public static final int PORT = 1857;
 public static void main(String[] args) {
 new Thread(new TextReceiver()).start();
 }
 @Override
 public void run() {
 ServerSocket serverSocket = null;
 Socket clientSocket = null;
 BufferedReader in = null;
 try {
 serverSocket = new ServerSocket(PORT);
 System.out.println("[RECEIVER] Waiting for a client.");
 clientSocket = serverSocket.accept();
 in = new BufferedReader(
 new InputStreamReader(clientSocket.getInputStream()));
 System.out.println("[RECEIVER] A client connected.");
 Scanner scanner = new Scanner(in);
 while (scanner.hasNextLine()) {
 System.out.println(scanner.nextLine());
 }
 } catch (IOException ex) {
 Logger.getLogger("CONNECTION").severe(ex.getMessage());
 if (in != null) {
 close(in, clientSocket, serverSocket);
 } else if (clientSocket != null) {
 close(clientSocket, serverSocket);
 } else if (serverSocket != null) {
 close(serverSocket);
 }
 }
 close(in, clientSocket, serverSocket);
 }
 private void close(Closeable... closeables) {
 for (Closeable c : closeables) {
 try {
 c.close();
 } catch (IOException ex) {
 }
 }
 }
}

Please, tell me anything I could improve.

asked Nov 3, 2015 at 12:48
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$
  1. Why define PORT as a constant independently in TextReceiver and TextSender? Isn't it the whole point that it's one and the same value? Let's stick to Single Source of Truth then. And if not, why make them public?
  2. I'm not sure if this is necessary:

    if (in != null) {
     close(in, clientSocket, serverSocket);
    } else if (clientSocket != null) {
     close(clientSocket, serverSocket);
    } else if (serverSocket != null) {
     close(serverSocket);
    }
    

    I'd rather have close skip over null values when iterating through closeables.

  3. It could be a better choice to use a finally clause for closing resources (in TextReceiver). It executes regardless of whether there was an exception or not.

  4. If we're logging all other exceptions, I can't see the reason not to log the IOException in close.

answered Nov 3, 2015 at 14:18
\$\endgroup\$
2
  • \$\begingroup\$ 1. I had to create two distinct projects for convenience. I know that repeating oneself ain't good. Otherwise all good points. \$\endgroup\$ Commented Nov 3, 2015 at 18:58
  • \$\begingroup\$ @coderodde oh right, it's client-server. I only use Java on Android so I'm not used to that. Well, but then - assuming PORT isn't referred to outside - I'd make it private. Thanks for accepting the answer. \$\endgroup\$ Commented Nov 3, 2015 at 19:36

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.