3
\$\begingroup\$

I'm fairly new at coding and looking to improve. Down below is all of my files for a TCP chatroom with a GUI in java. This is my first networking project so I'm sure it has many problems and does not scream clean code. Because of that I'm looking for some help to identify my blind spots and bad practices. If you have any questions or need clarification feel free to ask.

Server.java

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.*;
import java.util.ArrayList;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class Server implements Runnable {
    private ArrayList<ConnectionHandler> connections;
    private ServerSocket server;
    private boolean done;
    private ExecutorService threadPool;

    public Server() {
        connections = new ArrayList<>();
        done = false;
    }

    @Override
    public void run() {
        try {
            server = new ServerSocket(5001);
            threadPool = Executors.newCachedThreadPool();
            while (!done) {
                Socket client = server.accept();
                ConnectionHandler handler = new ConnectionHandler(client);
                connections.add(handler);
                threadPool.execute(handler);
            }

        } catch (Exception e) {
            shutdown();
        }
    }

    public void broadcast(String message) {
        for (ConnectionHandler ch : connections) {
            if (ch != null) {
                ch.sendMessage(message);
            }
        }
    }

    public void shutdown() {
        try {
            if (!server.isClosed()) {
                server.close();
            }
            for (ConnectionHandler ch : connections) {
                ch.shutdown();
            }
        } catch (IOException exception) {
            // ignore
        }
    }

    class ConnectionHandler implements Runnable {
        private Socket client;
        private BufferedReader input;
        private PrintWriter output;
        private String nickname;

        public ConnectionHandler(Socket client) {
            this.client = client;
        }
        @Override
        public void run() {
            try {
                output = new PrintWriter(client.getOutputStream(), true);
                input = new BufferedReader(new InputStreamReader(client.getInputStream()));

                output.println("Enter a nickname: ");
                nickname = input.readLine();
                broadcast(nickname + " has joined the chat.");

                String message;
                while ((message = input.readLine()) != null) {
                    if (message.startsWith("/nickname ")) {
                        String[] messageSplit = message.split(" ", 2);
                        if (messageSplit.length == 2) {
                            broadcast(nickname + " changed name to: " + messageSplit[1]);
                            nickname = messageSplit[1];
                        }
                    }
                    else if (message.startsWith("/quit")) {
                        broadcast(nickname + " has left the chat.");
                    } else {
                        broadcast(nickname + ": " + message);
                    }
                }

            } catch (IOException e) {
                shutdown();
            }

        }

        public void sendMessage(String message) {
            output.println(message);
        }

        public void shutdown() {
            try {
                input.close();
                output.close();
                if (!client.isClosed()) {
                    client.close();
                }
            } catch (IOException ex) {
                //ignore
            }
        }
    }
    public static void main(String[] args) {
        Server server = new Server();
        server.run();
    }
}

Client.java

import java.io.*;
import java.net.*;
public class Client implements Runnable {
    private Socket client;
    private BufferedReader input;
    private PrintWriter output;
    private boolean done;
    private ChatRoom chatRoom;

    @Override
    public void run() {
        try {
            client = new Socket("127.0.0.1", 5001);
            output = new PrintWriter(client.getOutputStream(), true);
            input = new BufferedReader(new InputStreamReader(client.getInputStream()));

            InputHandler inputHandler = new InputHandler();
            Thread thread = new Thread(inputHandler);
            thread.start();

            String inputMessage;
            while ((inputMessage = input.readLine()) != null) {
                System.out.println(inputMessage);
                chatRoom.receiveMessage(inputMessage);
            }
        } catch (IOException ex) {
            shutdown();
        }
    }
    public void setChatRoom(ChatRoom chatRoom) {
        this.chatRoom = chatRoom;
    }

    public void shutdown() {
        done = true;
        try {
            input.close();
            output.close();
            if (!client.isClosed()) {
                client.close();
            }
        } catch (IOException ex) {
            // ignore
        }
    }

    class InputHandler implements Runnable {
        @Override
        public void run() {
            try {
                while (!done) {
                    System.out.flush();
                    if (!chatRoom.message.isEmpty()) {
                        String message = chatRoom.message;
                        if (message.equals("/quit")) {
                            shutdown();
                        } else {
                            output.println(message); // Send message to the server
                        }
                    }
                    chatRoom.message = "";
                }
            } catch (Exception e) {
                shutdown();
            }
        }
    }

    public static void main(String[] args) {
        Client client = new Client();
        ChatRoom chatRoom = new ChatRoom();
        client.setChatRoom(chatRoom);
        chatRoom.startGUI();
        client.run();
    }
}

ChatRoom.java

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;

public class ChatRoom extends JFrame {
    private JTextArea chatArea;
    public JTextField inputField;
    public String message = "";

    public void startGUI() {
        setTitle("ChatRoom");
        setSize(400, 300);
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setLocationRelativeTo(null); // centering

        initUI();
        setVisible(true);
    }

    public void initUI() {

        setLayout(new BorderLayout());
        chatArea = new JTextArea();
        chatArea.setEditable(false);
        JScrollPane scrollPane = new JScrollPane(chatArea);
        add(scrollPane, BorderLayout.CENTER);
        JPanel inputPanel = new JPanel();
        inputField = new JTextField(25);
        JButton sendButton = new JButton("Send");

        sendButton.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                sendMessage();
            }
        });

        inputField.addKeyListener(new KeyAdapter() {
            @Override
            public void keyPressed(KeyEvent e) {
                if (e.getKeyCode() == KeyEvent.VK_ENTER) {
                    sendMessage();
                }
            }
        });

        inputPanel.add(inputField, BorderLayout.CENTER);
        inputPanel.add(sendButton, BorderLayout.EAST);
        add(inputPanel, BorderLayout.SOUTH);
    }

    public synchronized void sendMessage() {
        message = inputField.getText();
        if (!message.isEmpty()) {
            inputField.setText("");
        }
    }

    public void receiveMessage(String message) {
        SwingUtilities.invokeLater(() -> {
            chatArea.append(message + "\n");
        });
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Not a fan of the * wildcard imports, as they make it harder to read. I would expect the IDE to worry about such details for you. // It is astonishing that shutdown() does not accept an exception parameter, and optionally log what caused the shutdown. \$\endgroup\$
    – J_H
    Jan 12 at 2:54

1 Answer 1

2
\$\begingroup\$

Server:

Change:

private ArrayList<ConnectionHandler> connections;

to

private final List<ConnectionHandler> connections;


Same with the

private Socket client;

change it to :

private final Socket client;


Rename variable: catch (IOException ex)

to

catch (IOException ignore)


threadPool = Executors.newFixedThreadPool(N);

rather than:

Executors.newCachedThreadPool();

\$\endgroup\$

Your Answer

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

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