[Java] Network Class

[Ragehottie]

I sat down tonight and decided to do a little in java. Never really have done anything except some simple minecraft mods.
I like what turned out though


Here is the code(Criticism very welcome(Especially from Deque:P)):

Code: (java) [Select]




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


public class networking {

private static PrintWriter out;
private static BufferedReader in;
private static Socket server;

public void connect(String host, int port) throws IOException, NullPointerException {
server = new Socket(host, port);
out = new PrintWriter(server.getOutputStream(), true);
in = new BufferedReader(new InputStreamReader(server.getInputStream()));
}

public void disconnect() throws IOException, NullPointerException {
//System.out.println("Disconnecting...");
server.close();
out.close();
in.close();
//System.out.println("...Disconnected!");
}

public void send(String str) throws NullPointerException {
out.println(str);
}

public String read() throws IOException, NullPointerException {
String a = in.readLine();
return a;
}

public String listen() throws InterruptedException, IOException, NullPointerException {
String a = null;
while (a == null) {
Thread.sleep(1000);
a = in.readLine();
}
return a;
}
}
Basically it's just a cleaner way for sockets and such.

whatever.connect(host, port) -- to connect
whatever.send(message) -- to send to the server
whatever.read() -- read from the server
whatever.listen() -- stops and waits until it gets a message from the server
whatever.disconnect() -- to, well, disconnect



This is very simple, but it was fun to write!


Old Code:
http://pastebin.com/54pE0Xbj
http://pastebin.com/rgxzTpek

[Deque]

Will do.

Code: [Select]

static PrintWriter out = null;
 static BufferedReader in = null;
 static Socket server = new Socket();
Declare your fields as private. No need to initialize them. They are null by default and the socket is initialized in connect()

I suggest to throw the Exceptions in connect() and read(), because the caller should know if something happened (but I see, that you wanted to make that "cleaner" too).
Imagine the caller does this (without any System.exit(), I will elaborate later):

Code: [Select]

net.connect(host, port)
net.disconnect()
Not knowing that the connection failed, the caller will try to disconnect and raise a NullPointerException, because server, in and out may be null at this time.

Which is the reason you should additional check for null in disconnect()

And please, don't use System.exit() at all. It doesn't make sense to use it here. Maybe the caller doesn't want to exit, but try something else instead.
Just let the program "naturally" shut down itself. Before I elaborate more, I just cite myself:

System.exit() is bad. It shuts down the JVM regardless what runs within. If there is still a task, it will be shut down too. That makes it hard to reuse your code, to embed it and to make unit tests.

[Ragehottie]

Will do.

Code: [Select]

static PrintWriter out = null;
 static BufferedReader in = null;
 static Socket server = new Socket();
Declare your fields as private. No need to initialize them. They are null by default and the socket is initialized in connect()

I suggest to throw the Exceptions in connect() and read(), because the caller should know if something happened (but I see, that you wanted to make that "cleaner" too).
Imagine the caller does this (without any System.exit(), I will elaborate later):

Code: [Select]

net.connect(host, port)
net.disconnect()
Not knowing that the connection failed, the caller will try to disconnect and raise a NullPointerException, because server, in and out may be null at this time.

Which is the reason you should additional check for null in disconnect()

And please, don't use System.exit() at all. It doesn't make sense to use it here. Maybe the caller doesn't want to exit, but try something else instead.
Just let the program "naturally" shut down itself. Before I elaborate more, I just cite myself:

Thank you for the help. I (think I) did all of what you suggested. One thing I don't like what I did where I just printed the error directly. Is there some sort of method were I can just return the error to the caller?

[Deque]

Thank you for the help. I (think I) did all of what you suggested. One thing I don't like what I did where I just printed the error directly. Is there some sort of method were I can just return the error to the caller?

Remove the try-catch entirely.

[Ragehottie]

Remove the try-catch entirely.

Ah, so instead of the networking class handling teh exceptions, the caller does. Not as clean, but I like it.


Also, this just made my java understanding go up 100%. Thank ya