Асинхронный сетевой код обратного вызова


Я не получить работу после подачи этой работе в интервью, но у меня нет обратной связи, чтобы знать, что "плохие" вещи внутри этого блока кода.

Требования:

  • Подключиться к серверу на порт и IP
  • Асинхронно отправить сообщение на сервер на ваш выбор формат
  • Рассчитать и отобразить время приема для каждого сообщения и в то время для всех сообщений, отправляемых

Решение не должно быть так сложно. Но я просто не знаю, что случилось? Плохой дизайн? Плохих имен? Плохая практика?

import java.io.BufferedReader;
import java.io.DataOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.Socket;
import java.net.UnknownHostException;

public class EchoClient {

    private String hostname;
    private int port;
    private Socket clientSocket;
    private BufferedReader inFromUser, inFromServer;
    private DataOutputStream outToServer;
    private double averageTime = 0;
    private int count = 0;

    public EchoClient(String hostname, int port){
        this.hostname = hostname;
        this.port = port;
        try {
            this.clientSocket = new Socket(this.hostname, this.port);
        } catch (UnknownHostException e) {
            System.out.println("Connection Error: unknown host");
            System.exit(1);
        } catch (IOException e) {
            System.out.println("Connection Error: connection refused");
            System.exit(1);
        }
        try{
            this.inFromUser = new BufferedReader( new InputStreamReader(System.in));
            this.outToServer = new DataOutputStream(this.clientSocket.getOutputStream());
            this.inFromServer = new BufferedReader(
                    new InputStreamReader(this.clientSocket.getInputStream()));
        } catch (IOException e) {
            System.out.println("Error on Initializing echoclient");
            System.exit(1);
        }

    }

    public void start(){
        System.out.println("Connecting to " + hostname + " with port No " + port);
        String msgSend;
        try {
            while ((msgSend = inFromUser.readLine()) != null){
                // sendMessage asynchronous
                sendMessage(msgSend, new Callback(){
                    // callback function and calculate the average time
                    public void callback(long timeUsed, String msgReceived){
                        averageTime = (count * averageTime + (timeUsed)) / (count + 1);
                        ++count;
                        System.out.println(msgReceived + 
                            " rtt=" +  (double)Math.round(timeUsed * 100)/100    + " ms" +
                            " artt=" + (double)Math.round(averageTime * 100)/100 + " ms");

                    }
                });    
            }
        } catch (IOException e) {
            System.out.println("Error on reading message from user");
        }
    }

    private void sendMessage(String message, Callback cb){
        Thread sendMessageThread = new Thread(new SendMessageRequest(message, cb));
        sendMessageThread.start();
    }

    interface Callback {
        public void callback(long time, String msg);
    }

    class SendMessageRequest implements Runnable{

        private String message;
        private Callback cb;
        SendMessageRequest(String message, Callback cb){
            this.message = message;
            this.cb = cb;
        }
        @Override
        public void run() {
            String msgReceived;
            long timeStart, timeEnd, timeUsed;
            try {
                timeStart = System.nanoTime();
                outToServer.writeBytes(this.message + '\n');
                msgReceived = inFromServer.readLine();
                timeEnd = System.nanoTime();
                // Calculate the time and get the output
                timeUsed = (timeEnd - timeStart) / 1000000;
                cb.callback(timeUsed, msgReceived);
            } catch (IOException e) {
                System.out.println("Error on sending message to server");
            }

        }

    }

    public static void showUsage(){
        System.out.println("Usage: java EchoClient [hostname] [portNo]");
    }
    /**
     * Entry of the program
     */
            public static void main(String[] args) {
        String hostname = "";
        int port = 0;
        if (args.length < 2){
            showUsage();
            System.exit(0);
        }
        else{
            hostname = args[0];
            port = Integer.parseInt(args[1]);
        }

        EchoClient client = new EchoClient(hostname, port);
        client.start();
    }
}


Комментарии
2 ответа

Я думаю, что самой большой проблемой является отсутствие синхронизации. Вы изменить averageTime и считать переменные в обратного вызова, который проходит параллельно. Вы должны синхронизировать доступ к этой переменной. Есть хорошая книга на эту тему: читать на Java параллелизм на практике, если у вас есть время почитайте, это очень полезно.

Некоторые другие вещи:


  1. Я не люблю внутренние классы. Ссылка: эффективная Java второе издание, Пункт 22: пользу статические члены классов.
    Я бы также создать EchoClientMain класс, который содержит основной способ и разбора параметров командной строки. Кроме того, я хотел переехать в новый файл обратного вызова анонимного внутреннего класса, и я бы создать статистику класса, который будет отвечать за расчет и ведение статистики. (Отметьте один принцип ответственности в Википедии.)

  2. Вместо системы.выход() вы должны повторно сгенерировать исключение. Этот класс не многоразовые поскольку простая ошибка останавливает работу всего приложения. Просто создать пользовательский класс исключения и бросить его:

    try {
    this.clientSocket = new Socket(this.hostname, this.port);
    } catch (final UnknownHostException uhe) {
    throw new EchoClientException("Connection Error: unknown host", uhe);
    }

    Пусть абонент с ними справиться. В данном случае основной метод должен поймать EchoClientException и печатать свои сообщения на консоль.

    try {
    EchoClient client = new EchoClient(hostname, port);
    client.start();
    } catch (final EchoClientException ece) {
    System.err.println(ece.getMessage());
    }

  3. Вы должны не подключиться к серверу в конструкторе. Я сделаю это в начало() метод.

  4. Закрыть ресурсы. Создать стоп() метод, который закрывает открытые потоки.

  5. Проверить хотя бы на нулевые входные параметры: эффективная Java, пункт 38: проверка параметров на валидность

Чистый код Роберт С. Мартин тоже стоит почитать.

21
ответ дан 24 ноября 2011 в 07:11 Источник Поделиться

Интересно, когда они просили вас, чтобы отправить сообщение в асинхронном режиме, они не хочу, чтобы вы запустить второй поток. Ваш второй поток делает свою обработку синхронно. Да его на другой поток, но это может или не может быть то, что они имели ввиду под асинхронным. Что-то вроде библиотеки здесь: https://github.com/sonatype/async-http-client.

Как palacsint указывает, вы не синхронизировали переменных. Что мне делали его похожим вы не знаете, что вы делаете.

Другое дело мне не нравится, что способ обработки ошибок. Я закрыть приложение, если ошибка occours, не писать в стандартный поток ошибок и идти дальше. Я также хочу, чтобы отобразить больше информации об ошибке, не тот простой факт, что это произошло.

6
ответ дан 24 ноября 2011 в 08:11 Источник Поделиться