Sala de chat de múltiples hilos Java -- java campo con multithreading campo con swing campo con gui campo con chat camp codereview Relacionados El problema

Java Multi-threaded Chat Room


0
vote

problema

Español

En un intento de aprender más con Java, decidí desafiarme a la creación de una sala de chat con una GUI, con un flujo múltiple. Actualmente, ejecuta el cliente y le solicita que cree o se unirá a un servidor basado en una IP. El puerto utilizado es 9000. ¿Alguna sugerencia sobre cómo puedo mejorar?

server.java:

  import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.net.ServerSocket; import java.net.Socket; import java.sql.Timestamp; import java.util.HashSet; import java.util.Set;  public class Server extends Thread {     /**      * Used to write to all the connected clients.      */     private static final Set<PrintWriter> WRITERS = new HashSet<>();      /**      * Used to keep track of all of th enames of connected clients.      */     private static final Set<String> NAMES = new HashSet<>();      /**      * Used to keep track of the current client's ID for logging purposes.      */     private static int clientId = 1;      /**      * Used when creating a server.      */     private final ServerSocket listener;      public Server(ServerSocket listener) {         this.listener = listener;     }      /**      * Used to log messages.      *       * @param message      */     public static void log(String message) {         String timestamp = new Timestamp(System.currentTimeMillis()).toString();         timestamp = timestamp.substring(0, timestamp.length() - 4);         System.out.println("[" + timestamp + "] " + message);     }      /**      * Returns the ServerSocket listener.      *       * @return      */     public ServerSocket getListener() {         return this.listener;     }      @Override     public void run() {         if (listener != null) {             try {                 if (listener.isBound()) {                     log("Server started on " + (listener.getInetAddress().getHostAddress()) + " and listening on port "                             + ServerConstants.PORT);                 }                 while (true) {                     new User(listener.accept(), clientId++).start();                 }             } catch (IOException e) {                 e.printStackTrace();             } finally {                 try {                     listener.close();                 } catch (IOException e) {                     e.printStackTrace();                 }             }         }     }      /**      * Creates a server and starts accepting inputs.      *       * @param port      * @throws IOException      */     public static Server createServer(int port) throws IOException {         return new Server(new ServerSocket(port));     }      /**      * This class listens for and sends messages to messages from all clients.      *       * @author ivisc      *      */     private static class User extends Thread {         private Socket socket;         private PrintWriter out;         private BufferedReader in;         private String address;         private String name;         private int id;          public User(Socket sock, int id) {             this.socket = sock;             this.id = id;             this.address = socket.getInetAddress().getHostAddress();         }          @Override         public void run() {             try {                 log("Starting connection from " + this.toString());                 // Start up our variables for use with the socket.                 out = new PrintWriter(socket.getOutputStream(), true);                 in = new BufferedReader(new InputStreamReader(socket.getInputStream()));                  // Ask for a name                 while (true) {                     out.println("ENTERNAME");                     String line = in.readLine();                     if (line == null) {                         return;                     }                     if (!Filter.filter(line)) {                         out.println("INVALIDNAME");                     } else {                         synchronized (NAMES) {                             if (!NAMES.contains(line)) {                                 NAMES.add(line);                                 name = line;                                 out.println("NAMEACC");                                 break;                             }                         }                     }                 }                 WRITERS.add(out);                  log("Registering " + this.toString() + " (" + name + ")");                 for (PrintWriter writer : WRITERS) {                     writer.println("NEWMEM" + name);                 }                 // Now we listen for names                 while (true) {                     String message = in.readLine();                     if (message == null) {                         return;                     }                     if (!Filter.filter(message)) {                         out.println("INVALIDMESSAGE");                     } else {                         for (PrintWriter writer : WRITERS) {                             writer.println("MESSAGE" + name + ": " + message);                         }                     }                 }             } catch (IOException e) {                 log(this.toString() + " is disconnecting...");             } finally {                 log("Shutting down I/O from " + this.toString() + " (" + id + ")");                 try {                     out.close();                     in.close();                 } catch (IOException e) {                     e.printStackTrace();                 }             }         }          @Override         public String toString() {             return address + "(" + id + ")";         }     } }   

Client.Java:

  import java.awt.BorderLayout; import java.awt.Dimension; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; import java.net.Socket; import java.util.Scanner;  import javax.swing.JFrame; import javax.swing.JOptionPane; import javax.swing.JScrollPane; import javax.swing.JTextArea; import javax.swing.JTextField;  @SuppressWarnings("serial") public class Client extends JFrame {     private JTextField textField;     private JTextArea textArea;     private Socket socket;     private PrintWriter out;     private BufferedReader in;      public Client() {         textField = new JTextField(40);         getContentPane().add(textField, BorderLayout.SOUTH);         textField.setEditable(false);          textArea = new JTextArea();         getContentPane().add(textArea, BorderLayout.NORTH);         textArea.setEditable(false);         getContentPane().add(new JScrollPane(textArea), BorderLayout.CENTER);          textField.addActionListener(new ActionListener() {             @Override             public void actionPerformed(ActionEvent arg0) {                 out.println(textField.getText());                 textField.setText("");             }         });          setPreferredSize(new Dimension(800, 600));         pack();         setSize(getPreferredSize());         setLocationRelativeTo(null);         setVisible(true);         setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);     }      public void initVars() throws IOException {         this.out = new PrintWriter(socket.getOutputStream(), true);         this.in = new BufferedReader(new InputStreamReader(socket.getInputStream()));     }      public void connect(String address) {         try {             this.socket = new Socket(address, ServerConstants.PORT);             initVars();             this.textField.setEditable(true);             Server.log("Connected to " + address);             this.setVisible(true);             while (true) {                 new ServerListener(out, in).start();             }         } catch (IOException e) {             e.printStackTrace();         }     }      public void connect(Server server) {         try {             String address = server.getListener().getInetAddress().getHostAddress();             this.socket = new Socket(address, ServerConstants.PORT);             initVars();             this.textField.setEditable(true);             Server.log("Connected to " + address);             this.setVisible(true);             while (true) {                 new ServerListener(out, in).start();             }         } catch (IOException e) {             e.printStackTrace();         }     }      public static void main(String[] args) {         Scanner s = new Scanner(System.in);         Object[] options = new Object[] { "Start", "Join", "Cancel" };         int input = JOptionPane.showOptionDialog(null, "Would you like to create or join a server?",                 "Chat Room Selection", JOptionPane.YES_NO_CANCEL_OPTION, JOptionPane.INFORMATION_MESSAGE, null, options,                 options[1]);         Client client = new Client();         switch (input) {             case 0:                 try {                     Server server = Server.createServer(ServerConstants.PORT);                     server.start();                     client.connect(server);                 } catch (IOException e) {                     e.printStackTrace();                 }                 break;             case 1:                 String address = JOptionPane.showInputDialog("Please enter server IP");                 if (address != null) {                     client.connect(address);                 }                 break;             case 2:                 System.exit(0);                 break;         }         s.close();     }      private class ServerListener extends Thread {         private PrintWriter out;         private BufferedReader in;          public ServerListener(PrintWriter out, BufferedReader in) {             this.out = out;             this.in = in;         }          @Override         public void run() {             while (true) {                 try {                     handle(in.readLine());                 } catch (IOException e) {                     e.printStackTrace();                 }             }         }          public void handle(String message) throws IOException {             System.out.println("handling " + message);             if (message == null) {                 return;             }             System.out.println(message);             if (message.startsWith("MESSAGE")) {                 message = message.substring(7);                 textArea.append(message + " ");             } else if (message.startsWith("ENTERNAME")) {                 while (true) {                     String name = JOptionPane.showInputDialog("Please enter a username");                     out.println(name);                     String response = in.readLine();                     if (response.equals("INVALIDNAME")) {                         JOptionPane.showMessageDialog(null, "Name does not pass Name Filter", "Invalid Name",                                 JOptionPane.ERROR_MESSAGE);                     } else {                         break;                     }                 }             } else if (message.startsWith("NEWMEM")) {                 String name = message.substring(6);                 textArea.append(name + " has joined the server. ");             }         }     } }   

Hay algunas referencias a serverConstants.Java y filter.java aquí, pero no los incluiré para el bien de Brevity. Básicamente, ServerConstants es una interfaz abstracta que simplemente define el puerto para usar, y el filtro es un filtro de nombre simple utilizando una lista de "malas palabras" (creo que es este que usé).

Sé que no comenté todo, y agregando throws765544337 no puede ser la mejor práctica, y generalmente no se recomienda la extensión de JFrame8 , pero, en función de todo Aquí, ¿alguna sugerencia?

Original en ingles

In an attempt to learn more with Java, I decided to challenge myself by creating a GUI-based, multi-threaded chat room. Currently, you run the Client and it prompts you to either create or join a server based on an IP. Port used is 9000. Any suggestions on how I can improve?

Server.java:

import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.net.ServerSocket; import java.net.Socket; import java.sql.Timestamp; import java.util.HashSet; import java.util.Set;  public class Server extends Thread {     /**      * Used to write to all the connected clients.      */     private static final Set<PrintWriter> WRITERS = new HashSet<>();      /**      * Used to keep track of all of th enames of connected clients.      */     private static final Set<String> NAMES = new HashSet<>();      /**      * Used to keep track of the current client's ID for logging purposes.      */     private static int clientId = 1;      /**      * Used when creating a server.      */     private final ServerSocket listener;      public Server(ServerSocket listener) {         this.listener = listener;     }      /**      * Used to log messages.      *       * @param message      */     public static void log(String message) {         String timestamp = new Timestamp(System.currentTimeMillis()).toString();         timestamp = timestamp.substring(0, timestamp.length() - 4);         System.out.println("[" + timestamp + "] " + message);     }      /**      * Returns the ServerSocket listener.      *       * @return      */     public ServerSocket getListener() {         return this.listener;     }      @Override     public void run() {         if (listener != null) {             try {                 if (listener.isBound()) {                     log("Server started on " + (listener.getInetAddress().getHostAddress()) + " and listening on port "                             + ServerConstants.PORT);                 }                 while (true) {                     new User(listener.accept(), clientId++).start();                 }             } catch (IOException e) {                 e.printStackTrace();             } finally {                 try {                     listener.close();                 } catch (IOException e) {                     e.printStackTrace();                 }             }         }     }      /**      * Creates a server and starts accepting inputs.      *       * @param port      * @throws IOException      */     public static Server createServer(int port) throws IOException {         return new Server(new ServerSocket(port));     }      /**      * This class listens for and sends messages to messages from all clients.      *       * @author ivisc      *      */     private static class User extends Thread {         private Socket socket;         private PrintWriter out;         private BufferedReader in;         private String address;         private String name;         private int id;          public User(Socket sock, int id) {             this.socket = sock;             this.id = id;             this.address = socket.getInetAddress().getHostAddress();         }          @Override         public void run() {             try {                 log("Starting connection from " + this.toString());                 // Start up our variables for use with the socket.                 out = new PrintWriter(socket.getOutputStream(), true);                 in = new BufferedReader(new InputStreamReader(socket.getInputStream()));                  // Ask for a name                 while (true) {                     out.println("ENTERNAME");                     String line = in.readLine();                     if (line == null) {                         return;                     }                     if (!Filter.filter(line)) {                         out.println("INVALIDNAME");                     } else {                         synchronized (NAMES) {                             if (!NAMES.contains(line)) {                                 NAMES.add(line);                                 name = line;                                 out.println("NAMEACC");                                 break;                             }                         }                     }                 }                 WRITERS.add(out);                  log("Registering " + this.toString() + " (" + name + ")");                 for (PrintWriter writer : WRITERS) {                     writer.println("NEWMEM" + name);                 }                 // Now we listen for names                 while (true) {                     String message = in.readLine();                     if (message == null) {                         return;                     }                     if (!Filter.filter(message)) {                         out.println("INVALIDMESSAGE");                     } else {                         for (PrintWriter writer : WRITERS) {                             writer.println("MESSAGE" + name + ": " + message);                         }                     }                 }             } catch (IOException e) {                 log(this.toString() + " is disconnecting...");             } finally {                 log("Shutting down I/O from " + this.toString() + " (" + id + ")");                 try {                     out.close();                     in.close();                 } catch (IOException e) {                     e.printStackTrace();                 }             }         }          @Override         public String toString() {             return address + "(" + id + ")";         }     } } 

Client.java:

import java.awt.BorderLayout; import java.awt.Dimension; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; import java.net.Socket; import java.util.Scanner;  import javax.swing.JFrame; import javax.swing.JOptionPane; import javax.swing.JScrollPane; import javax.swing.JTextArea; import javax.swing.JTextField;  @SuppressWarnings("serial") public class Client extends JFrame {     private JTextField textField;     private JTextArea textArea;     private Socket socket;     private PrintWriter out;     private BufferedReader in;      public Client() {         textField = new JTextField(40);         getContentPane().add(textField, BorderLayout.SOUTH);         textField.setEditable(false);          textArea = new JTextArea();         getContentPane().add(textArea, BorderLayout.NORTH);         textArea.setEditable(false);         getContentPane().add(new JScrollPane(textArea), BorderLayout.CENTER);          textField.addActionListener(new ActionListener() {             @Override             public void actionPerformed(ActionEvent arg0) {                 out.println(textField.getText());                 textField.setText("");             }         });          setPreferredSize(new Dimension(800, 600));         pack();         setSize(getPreferredSize());         setLocationRelativeTo(null);         setVisible(true);         setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);     }      public void initVars() throws IOException {         this.out = new PrintWriter(socket.getOutputStream(), true);         this.in = new BufferedReader(new InputStreamReader(socket.getInputStream()));     }      public void connect(String address) {         try {             this.socket = new Socket(address, ServerConstants.PORT);             initVars();             this.textField.setEditable(true);             Server.log("Connected to " + address);             this.setVisible(true);             while (true) {                 new ServerListener(out, in).start();             }         } catch (IOException e) {             e.printStackTrace();         }     }      public void connect(Server server) {         try {             String address = server.getListener().getInetAddress().getHostAddress();             this.socket = new Socket(address, ServerConstants.PORT);             initVars();             this.textField.setEditable(true);             Server.log("Connected to " + address);             this.setVisible(true);             while (true) {                 new ServerListener(out, in).start();             }         } catch (IOException e) {             e.printStackTrace();         }     }      public static void main(String[] args) {         Scanner s = new Scanner(System.in);         Object[] options = new Object[] { "Start", "Join", "Cancel" };         int input = JOptionPane.showOptionDialog(null, "Would you like to create or join a server?",                 "Chat Room Selection", JOptionPane.YES_NO_CANCEL_OPTION, JOptionPane.INFORMATION_MESSAGE, null, options,                 options[1]);         Client client = new Client();         switch (input) {             case 0:                 try {                     Server server = Server.createServer(ServerConstants.PORT);                     server.start();                     client.connect(server);                 } catch (IOException e) {                     e.printStackTrace();                 }                 break;             case 1:                 String address = JOptionPane.showInputDialog("Please enter server IP");                 if (address != null) {                     client.connect(address);                 }                 break;             case 2:                 System.exit(0);                 break;         }         s.close();     }      private class ServerListener extends Thread {         private PrintWriter out;         private BufferedReader in;          public ServerListener(PrintWriter out, BufferedReader in) {             this.out = out;             this.in = in;         }          @Override         public void run() {             while (true) {                 try {                     handle(in.readLine());                 } catch (IOException e) {                     e.printStackTrace();                 }             }         }          public void handle(String message) throws IOException {             System.out.println("handling " + message);             if (message == null) {                 return;             }             System.out.println(message);             if (message.startsWith("MESSAGE")) {                 message = message.substring(7);                 textArea.append(message + "\n");             } else if (message.startsWith("ENTERNAME")) {                 while (true) {                     String name = JOptionPane.showInputDialog("Please enter a username");                     out.println(name);                     String response = in.readLine();                     if (response.equals("INVALIDNAME")) {                         JOptionPane.showMessageDialog(null, "Name does not pass Name Filter", "Invalid Name",                                 JOptionPane.ERROR_MESSAGE);                     } else {                         break;                     }                 }             } else if (message.startsWith("NEWMEM")) {                 String name = message.substring(6);                 textArea.append(name + " has joined the server.\n");             }         }     } } 

There are a few references to ServerConstants.java and Filter.java here, but I won't include them for brevity's sake. Basically, ServerConstants is an abstract interface that just defines the port to use, and Filter is a simple name filter using a list of "bad words" (I believe it's this one that I used).

I know I didn't comment everything, and adding throws statements might not be best practice, and extending from JFrame is usually not recommended, but, based on everything here, any suggestions?

              

Lista de respuestas

3
 
vote

En primer lugar, es bueno ver al menos algunos comentarios. Hace que la comprensión de su código sea mucho más fácil.

a las sugerencias:
Sí, tienes razón, heredándome de JFrame no es bueno. Heredando del hilo no es bueno, también. Su caso es un poco peor porque su código dice, su cliente es un JFrame, su servidor es un hilo y su usuario también es un hilo. Eso es realmente confuso. El problema es que está mezclando la lógica de su dominio con las cosas técnicas.
Pregúntese esto: ¿Qué quiere que su programa haga (desde el punto de vista de los usuarios)? Yo diría que quiere tener usuarios que puedan enviar mensajes. Eso es algo que asumiría encontrar en su clase de usuario.
La GUI y los hilos son algo completamente diferentes. Están en una capa diferente y su clase de usuario no debería tener ningún conocimiento sobre ellos.

Además, sus clases están haciendo demasiado y no es fácil predecir quién hará qué. Debe preguntarse antes de que estés creando un método: ¿Cuál es su trabajo? ¿Cuál será la salida de este método y cómo lo probaría?

Tome esto, por ejemplo:

  limitToRange5  

Este método sería realmente difícil de probar porque hace cosas múltiples, muy diferentes y porque tiene dos dependencias duras: Socket y ServerListener. Si no tiene una conexión a Internet, su examen fallaría. Si su Lista de servidores falla, su prueba también falla. Trate de deshacerse de tales dependencias y déjelos tomar interfaces en lugar de clases. De esta manera, podría implementar una versión alternativa de la clase para usarla en las pruebas.

Además, sus dos clases de conexión se ven casi idénticas. Debe crear un método para la lógica superpuesta.

y finalmente:

  limitToRange6  

Está creando un objeto en un bucle infinito y supongo que dejará el bucle lanzando y captando una excepción, ¿verdad? No hagas eso. Las excepciones son para casos excepcionales y no para controlar el flujo. Utilizar IFS en su lugar.

 

First of all, it's nice to see at least some comments. It makes understanding your code a lot easier.

To the suggestions:
Yes, you are right, inheriting from JFrame isn't good. Inheriting from Thread isn't good, too. Your case is a little bit worse because your code says, your client is a JFrame, your server is a thread and your user is a thread, too. That's really confusing. The problem is, that you are mixing your domain logic with the technical stuff.
Ask yourself this: What do you want your program to do (from a users standpoint)? I would say you want to have users that can send each other messages. That's something I would assume to find in your User class.
The GUI and the threads are something completely different. They are on a different layer and your user class shouldn't have any knowledge about them.

Additionally, your classes are doing too much and it's not easy to predict who will do what. You should ask yourself before you're creating a method: What's its job? What will be the output of this method and how would I test it?

Take this for example:

    public void connect(String address) {         try {             this.socket = new Socket(address, ServerConstants.PORT);             initVars();             this.textField.setEditable(true);             Server.log("Connected to " + address);             this.setVisible(true);             while (true) {                 new ServerListener(out, in).start();             }         } catch (IOException e) {             e.printStackTrace();         }     } 

This method would be really hard to test because it does multiple, very different things and because you have two hard dependencies: Socket and ServerListener. If you don't have an internet connection, your test would fail. If your ServerListener fails, your test fails, too. Try to get rid of such dependencies and let them take interfaces instead of classes. This way you could implement an alternative version of the class to use it in the tests.

Besides, your two connect classes look almost identical. You should create a method for overlapping logic.

And finally:

    while (true) {         new ServerListener(out, in).start();     } 

You are creating an object in an infinite loop and I guess you will leave the loop by throwing and catching an exception, right? Don't do that. Exceptions are for exceptional cases and not to control the flow. Use ifs instead.

 
 

Relacionados problema

6  Sala de chat en tiempo real  ( Realtime chat room ) 
Esta es una sala de chat realmente simple que he hecho con Firsebase. Antes de llevarlo al siguiente nivel, quiero saber si hay algo que pueda mejorar ahora m...

4  Sala de chat de zócalo hecha con Tkinter Python  ( Socket chat room made with tkinter python ) 
Esta es una sala de chat real real que he hecho con Tkinter Publiqué una pregunta antes de hacer una sala de chat, pero el problema que tenía con ese era qu...

1  groupme chat bot  ( Groupme chat bot ) 
Este es mi segundo proyecto serio usando PHP, hice un BOTME BOT antes, pero el único PHP fue escribir los datos de publicación a un archivo y ejecutar un scri...

3  MEF Ejecutar complementos Exportaciones en hilos  ( Mef execute plugin exports in threads ) 
Estoy desarrollando una solicitud de servicio que implementaría un sistema de mensajería de igual a igual. Me pregunto si mi siguiente código es el mejor enfo...

2  Solicitud de chat  ( Chat application ) 
He escrito un backend para una aplicación de chat, pero no estoy seguro, si estoy siguiendo todos los principios sólidos y si mi código está lo suficientement...

10  Insignia Oneboxer para chat.se  ( Badge oneboxer for chat se ) 
Este script hace posible que las insignias sean con chat. La instalación requiere TAMPER / GRAASMONEYKEY. Haga clic en aquí para instalar, y Está en ...

2  Bot de registro de IRC  ( Irc logging bot ) 
Recientemente he estado aprendiendo Python y decidí escribir un Bot IRC como un buen primer proyecto. Antes de ahora, solo he escrito realmente scripts. Est...

8  Comprobando algunas reglas antes de que un Bot de telegrama responda a un mensaje  ( Checking some rules before a telegram bot replies to a message ) 
Hasta ahora, he creado un par de programas, y el problema que aparece es lo feo y no escalable que se ven. Por ejemplo, actualmente estoy trabajando en una ...

2  Bot de chat imprsimple escrito en Python  ( Imprsimple chat bot written in python ) 
Me gustaría saber si puedo mejorar el rendimiento de mi bot recientemente, y tal vez también algunos patrones de diseño. Hasta ahora, es advertencia de un u...

7  Simple Slack Bot en Python  ( Simple slack bot in python ) 
Escribí un simple bot slack, que puede guardar enlaces (comando: "Guardar nombre de nombre") e imprimirlos (comando: "Obtener nombre"). Opcionalmente, se pued...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos