Server side of a chat program

This is just the server part of a chat program. The user of the server can receive and send messages to all the clients connected to the server. Everything works fine but since I'm a beginner I don't know if I should avoid something or not, what I should improve, etc..

The server class of the app:

```
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.HashMap;
import java.util.Map;

public class ServerSide {
private static ServerSocket ss;
private static Socket sock;
private int port, ID;
private static String textReceived;
private static Map clientList = new HashMap();

public ServerSide() {

}
// Main Constructor takes the port to connect to a server
// in order to chat with the clients
public ServerSide(int port) throws IOException {
this.port = port;
ss = new ServerSocket(port);

Thread t1 = new Thread(new Runnable() {

@Override
public void run() {

while (true) {
try {
sock = ss.accept();
//Every client has an unique ID that will be use to send the receiving messages
//to all the clients except to the sender
ID++;
clientList.put(new Client(sock, ID), ID);
} catch (IOException e) {
e.printStackTrace();
}
}
}
});
t1.start();

}

public Map getClientsList() {
return clientList;
}
//This method is used everytime the user of the server clicks on "Send".
protected void send(String textSent) throws IOException {
new SendMessages(clientList, textSent);

}
//Return the text to append on the TextArea of the chat.
public String getTextReceived() {
return textReceived;
}

public void setTextReceived(String textReceived) {
this.textReceived = textReceived;
}
}

class SendMessages

Default Constructors:

public ServerSide() {

}

So, you want a default constructor, but you don't instantiate the class in it? Given your primary constructor (ServerSide(int port)), your default constructor should probably call this constructor with the default port. If you do not have a good default port, you probably shouldn't have this constructor. If your class needs a port number to work correctly, then you really need to remove this constructor.

main()

I don't like how you do all your work in main(). Move your work to dedicated, reusable methods that do a single thing. main()'s job is to start the program, not to do all the work.

Government Issue?

private static GraphicSide gi =  new GraphicSide();

Please use understandable naming. When I saw gi used in main(), I had to go back to find what it was really doing. Give it a name that specifies that it is the graphics side of the program right in the name. The same applies to various other names in the program.

Field Declarations

private int port, ID;

Just because you can declare multiple fields/variables with a single statement doesn't mean it is a good idea. In fact, it should never be used, in my non-so-humble opinion, unless the values are closely related, like the x and y coordinates of a chess piece on a board (and maybe not even then). I'm not sure if the port and the ID are closely related enough to be placed in the same declaration construct.

One Ring to Rule them All

static fields--one instance of these fields across each instance of the class. Every time you assign a new value to one of these fields, each class starts working with the new value. This is probably a bug you will need to fix.

OOP

In a comment you state "I add a default costructor only to get getClientsList() and set setTextReceived()." This is not how instances of classes in OOP work. If, and only if, these methods should have exactly the same result when called from any instance (or even a non-instance) of a class, then you can make them static. This way, you will not need an instance of the class to work with these. However, you need a chat connection to send messages, so I don't think this will work.

OOP does not usually work like that. In OOP, you create instances of classes that have nothing to do with each other. For example, if you have a Dog class, you can't just call Dog.Bark() without an instance--you can't make a non-existent dog bark. First, you create an instance, then you make the instance bark.

Dog dog = new Dog();
dog.bark();

In this program, you need to create an instance of a server-side chat instance that contains all its state. You cannot access elements of this class's state just by instantiating another instance--you need a reference to the instance. Once you understand how OOP works a little better, you will understand why you don't create default constructors like this, and why you should avoid static fields, methods, and classes.

Thread Safety

Your threading is so messed up I don't even know how messed up it is myself. Sorry if I seem harsh, but threading is very, very hard to get right, so please understand I'm not blaming you.

For example, you are adding clients in one thread here:

Thread t1 = new Thread(new Runnable() {

    @Override
    public void run() {

        while (true) {
            try {
                sock = ss.accept();
                //Every client has an unique ID that will be use to send      the receiving messages
                //to all the clients except to the sender
                ID++;
                clientList.put(new Client(sock, ID), ID);
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
});
t1.start();

Then, you take this list from another thread when you send a message and pass it to a new SendMessages instance:

protected void send(String textSent) throws IOException {
    new SendMessages(clientList, textSent);

}

In SendMessages, you iterate that list:

public SendMessages(Map list1, String textSent, int ID) throws IOException {
    // ...

    for (Map.Entry con : list1.entrySet()) {
        // ...
    }
}

It is never, ever a good idea to access or modify data in two threads at once. See the famous Producer/Consumer and Dining Philosophers problems as a partial example of this. You need to add proper access control so only a single thread accesses this at a given time; look up semaphores and mutexes, for example.

StackExchange Code Review Q#148393, answer score: 5