Can't get past the last requirement

Question about the task

Resolved

The issue is that the requirements aren't really that clear what message they want. I will note that I have done something a little bit different in how I approached this; I've nested try/catch statements in order to be able to differentiate whether the exception comes from the Connection constructor or the serverHandshake method.

It's time to write the main method of the Handler class, which will call all
the helper methods we wrote earlier. Implement the void run() method in the Handler class.

It must:
1) Display a message indicating that a new connection was established with a remote
address that can be obtained using the getRemoteSocketAddress() method.
2) Create a Connection object using the socket field.
3) Call the method that implements the handshake with the client, and store the name of the new client.
4) Send the name of the new participant (USER_ADDED message) to all chat participants.
Think about which method is most suitable for this.
5) Inform the new participant about the existing participants.
6) Start the server's main message-processing loop.
7) Be sure that the connection is closed if an exception occurs.
8) If any IOExceptions or ClassNotFoundExceptions occur, catch them and display a message indicating
that an error occurred while communicating with the remote address. 9) After handling any exceptions, if step 11.3 has finished and returned a name, we need to remove
from connectionMap the entry for that name and send a USER_REMOVED message containing the name to all other participants.
10) The last thing we need to do in the run() method is display a message
indicating that the connection with the remote address is closed.

Our server is completely ready. Try running it.

Requirements:

The run() method should display a message indicating that a connection has been established with the remote address (use the getRemoteSocketAddress() method).
The run() method should create a new connection using the socket field as an argument.
The run() method should call the serverHandshake() method using the newly created connection as an argument; the return value will be the username (userName).
The run() method should call the sendBroadcastMessage() method using a new message as an argument (MessageType.USER_ADDED, userName).
The run() method should call the notifyUsers() method using connection and userName as arguments.
The run() method should call the serverMainLoop() method using connection and userName as arguments.
Before completing, the run() method must remove from connectionMap the entry that corresponds to userName, and send a message to all chat participants that the current user has been removed.
The run() method should correctly handle IOExceptions and ClassNotFoundExceptions.

package com.codegym.task.task30.task3008; import java.io.Console; import java.io.IOException; import java.net.ServerSocket; import java.net.Socket; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; public class Server { private static Map<String, Connection> connectionMap = new ConcurrentHashMap<>(); public static void main(String[] args) { int serverPort = ConsoleHelper.readInt(); try(ServerSocket serverSocket = new ServerSocket(serverPort)) { ConsoleHelper.writeMessage("The server is running."); Socket socket; while(true) { socket = serverSocket.accept(); new Handler(socket).start(); } } catch(IOException e) { e.printStackTrace(); } } private static class Handler extends Thread { Socket socket; String userName; public void run() { ConsoleHelper.writeMessage("Connection established with " + socket.getRemoteSocketAddress()); try(Connection connection = new Connection(socket)) { try { userName = serverHandshake(connection); sendBroadcastMessage(new Message(MessageType.USER_ADDED, userName)); notifyUsers(connection, userName); serverMainLoop(connection, userName); connectionMap.remove(userName); sendBroadcastMessage(new Message(MessageType.USER_REMOVED, userName)); } catch(Exception e) { ConsoleHelper.writeMessage("Failed to get username from connection"); connection.close(); } } catch(IOException e) { ConsoleHelper.writeMessage("Failed to create new Connection using socket."); } sendBroadcastMessage(new Message(MessageType.TEXT, "The connection with the remote address is closed.")); } public Handler(Socket socket) { this.socket = socket; } private String serverHandshake(Connection connection) throws IOException, ClassNotFoundException { while(true) { connection.send(new Message(MessageType.NAME_REQUEST)); Message response = connection.receive(); if(response.getType() == MessageType.USER_NAME) { if(response.getData() != "" && !connectionMap.containsKey(response.getData())) { connectionMap.put(response.getData(), connection); connection.send(new Message(MessageType.NAME_ACCEPTED)); return response.getData(); } } } } private void notifyUsers(Connection connection, String userName) throws IOException { for(Map.Entry<String, Connection> entry : connectionMap.entrySet()) { if(userName != entry.getKey()) { connection.send(new Message(MessageType.USER_ADDED, entry.getKey())); } } } private void serverMainLoop(Connection connection, String userName) throws IOException, ClassNotFoundException { Message message; while(true) { message = connection.receive(); if(message.getType() == MessageType.TEXT) { sendBroadcastMessage(new Message(MessageType.TEXT, userName + ": " + message.getData())); } else { ConsoleHelper.writeMessage("Error: MessageType not equal to TEXT"); } } } } public static void sendBroadcastMessage(Message message) { for(Map.Entry<String, Connection> entry : connectionMap.entrySet()) { try { entry.getValue().send(message); } catch(IOException e) { System.out.println("The message could not be sent to " + entry.getKey()); } } } }

Comments (6)

Popular
New
Old

You must be signed in to leave a comment

Ibrahim

Level 41 , Sheffield, United Kingdom

18 March 2022, 08:31

It doesn't use a try-with-resources and has only one try block for catching the IOException and the ClassNotFoundException. I then close the connection in the finally block. I think this is what the validator wants from you.

Ibrahim

Level 41 , Sheffield, United Kingdom

18 March 2022, 08:29

Here is my code which passed:

@Override
        public void run() {
            ConsoleHelper.writeMessage("" + socket.getRemoteSocketAddress());

            Connection connection = null;
            String userName = "";

            try {
                connection = new Connection(socket);
                userName = serverHandshake(connection);



                serverHandshake(connection);

                sendBroadcastMessage(new Message(MessageType.USER_ADDED, userName));

                notifyUsers(connection, userName);

                serverMainLoop(connection, userName);

                connectionMap.remove(userName);

                sendBroadcastMessage(new Message(MessageType.USER_REMOVED, userName));

            } catch (IOException | ClassNotFoundException e) {
                ConsoleHelper.writeMessage(e.getMessage());
            } finally {
                try {
                    if (connection != null){
                        connection.close();
                        ConsoleHelper.writeMessage("connection closed");
                    }
                } catch (IOException io){
                    ConsoleHelper.writeMessage(io.getMessage());
                }
            }
        }
    }

Lisa L

Level 47 , Nuremberg, Germany

18 March 2022, 00:21

solution

You could try to not catch an Exception for the inner try block in the run method. That'll catch IOExceptions as well. Stick to what CG asks for and catch a ClassNotFoundException. That already could be enough to pass the task. But you even won't need nested try blocks. Just use the try-with-resources one and add two catch blocks (or a multicatch block) try (Connection...) { } catch (Ex1 e1) { } catch (Ex2 e2) { } or } catch (Ex1 | Ex2 e) { } closing the connection is also redundant... that's doing the try-with-resources. And if you really insist on doing so you'd have to better do that in the IOException catch block. That'll probably need a try-catch clock there, too. That's why you use try-with-resources

Justin Smith

Level 41 , Greenfield, USA, United States

18 March 2022, 01:24

I should mention that I'm posting this from the perspective of having looked at the solution. Usually when I feel confident about my code, and it runs without error, but still won't pass a validation, I'll look at the solution to try and determine if it's just a case of CodeGym stubbornly looking for a specific structure of code. As I mentioned, I wanted to code it with nested try blocks because if the try block catches an IOException, the way the solution is written you can't tell if it was new Connection or serverHandshake that triggered the IOException (they both throw IOException), and that seems like bad form to me. So I'm trying to see if I can get it to pass while preserving the ability to discern where the IOException originates. The way I have nested the two try/catch blocks should cause the two different IOExceptions to go to different catch blocks, allowing different messages to be sent for each circumstance. Sometimes CodeGym's validation is based purely on result, and other times it's based on looking for a specific line of code (sometimes so specific as to require a specific name for a variable), and sometimes it's a mix of both. I'm unsure in this case which it is. The connection.close() is indeed redundant. Even if the outer try succeeds without exception, it will still close the resource when exiting the block. I've removed that.

Lisa L

Level 47 , Nuremberg, Germany

18 March 2022, 10:13

Yes, my point was you using Exception... catches both of the required exceptions and other possible ones while you just should pick out the two mentioned (my first two sentence above if it was unclear). And with using two catch blocks on one try block you could omit the nested try blocks but that's not really necessary to do. It just looks cleaner. Multicatch is just another possibility (that's why I mentioned it last).

Justin Smith

Level 41 , Greenfield, USA, United States

18 March 2022, 11:19

I ended up making a few changes. The structure of the code most closely resembles yours so I marked yours as the solution. One of the things I also changed was to have it write the exception getMessage() method. It's possible the validation was looking for that. I initially didn't want to do that because this was meant as a chat client and I didn't want to give the user a raw error message.