Is this bad code? (streams, try - catch - finally and exceptions)

[Bladtman242]

The code executes as expected, but I am not sure it is considered good code:)
I am thinking especially of the two streams on the socket, if the first one is successfully opened and the next one for some reason throws an exception,
the finally block will never run.

If someone would validate whether or not this code is okay, I would really appreciate it
:)

import java.io.*;
import java.net.*;

public class EchoClient {
	public static void main(String[] args) throws IOException {
		
		Socket clientSocket = null;
		PrintWriter out = null;
		BufferedReader in = null;
		
		try {
		    clientSocket = new Socket("192.168.0.104", 7);
		    out = new PrintWriter(clientSocket.getOutputStream(), true);
		    in = new BufferedReader(new InputStreamReader(clientSocket.getInputStream()));
		    	
		    	try {
		             BufferedReader userInBuffer= new BufferedReader(new InputStreamReader(System.in));

		    	     String userInput;
		    
		   	     if ((userInput = userInBuffer.readLine()) != null) {
		    	     out.println(userInput);
		    	     System.out.println("echo: " + in.readLine());
		             }
		        
		        } finally {
		                  out.close();
		    		  in.close();
		    		  clientSocket.close();
		    	}
		    
		} catch (UnknownHostException e) {
		    	System.err.println("Host name unavailable.");
		    	System.exit(1);
		} catch (IOException e) {
		    	System.err.println("Couldn't get I/O for the connection."); 
		    	System.exit(1);
		}	   
		    
	}
}

[NormR1]

If someone would validate whether or not this code is okay

How do we tell if it is ok? What are the criteria?

Can you explain what the code is supposed to do? I can't see any comments in the code describing what it does.

[Bladtman242]

I'm sorry, I completely forgot to write that. :)
The program is very simple (I wrote for learning purposes only)
It sends a message to an echo server on my LAN and then writes the answer from the server to the std output.

What I meant with 'okay' was if I have done something that is considered bad coding.
Furthermore I am in doubt whether or not my try-catch statements are doing what they are supposed to = making sure the finally block closes the streams if they have been successfully opened :)

EDIT: I can't edit the first post, so here is the code again, this time with a few comments :)

import java.io.*;
import java.net.*;

public class EchoClient {
	public static void main(String[] args) throws IOException {
		
		Socket clientSocket = null;
		PrintWriter out = null;
		BufferedReader in = null;
		
		try {
                    //Creating a socket and both an input and an output stream on it
		    clientSocket = new Socket("192.168.0.104", 7);
		    out = new PrintWriter(clientSocket.getOutputStream(), true);
		    in = new BufferedReader(new InputStreamReader(clientSocket.getInputStream()));
		    	
		    	try {
                             //receiving a string from the user via command line
                             //and writing the users input to the outputstream on the socket.
		             BufferedReader userInBuffer= new BufferedReader(new InputStreamReader(System.in));

		    	     String userInput;
		    
		   	     if ((userInput = userInBuffer.readLine()) != null) {
		    	     out.println(userInput);
		    	     System.out.println("echo: " + in.readLine());
		             }
		        
		        } finally {
                                  //*hopefully* making sure that any opened stream is closed, no matter what.
                                  //but I am afraid that if the first stream is opened and the second one fails for some reason,
                                  //the program will still try and close the second stream, which wouldn't have been opened.
		                  out.close();
		    		  in.close();
		    		  clientSocket.close();
		    	}
		//cathing exceptions, but I think this might be a bad way of doing it, as both the first the later try block 
                //could properly throw an IOException, and this way there is no telling where the program fails (if it fails)
		} catch (UnknownHostException e) {
		    	System.err.println("Host name unavailable.");
		    	System.exit(1);
		} catch (IOException e) {
		    	System.err.println("Couldn't get I/O for the connection."); 
		    	System.exit(1);
		}	   
		    
	}
}

[NormR1]

making sure the finally block

Add a println() to see if/when it is executed

[Bladtman242]

I'm not doing a very good job making myself clear am I:)
It is executing the finally block, because the program runs as expected.
But I am afraid that if an error should occur, the program might not handle it
very well :)

e.g. if the first stream is opened but the second stream fails, the program will try and close both streams, hence making a call to an object that does not exist.

Does this matter, or am I obsessing over nothing?
:)

[NormR1]

making a call to an object that does not exist.

Use an if to test if the object is null.

[Bladtman242]

Ha ha, can't believe I didn't think of that :)
Well, thanks for your help :)