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

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

Требования:

  
  • Подключение к серверу на известном порту и 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();
    }
}
32 голоса | спросил Kit Ho 24 42011vEurope/Moscow11bEurope/MoscowThu, 24 Nov 2011 21:21:42 +0400 2011, 21:21:42

2 ответа


21

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

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

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

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

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

    Позвольте вызывающему устройству обработать их. В этом случае метод main должен поймать EchoClientException и распечатать его сообщение на консоли.

    try {
        EchoClient client = new EchoClient(hostname, port);
        client.start();
    } catch (final EchoClientException ece) {
        System.err.println(ece.getMessage());
    }
    
  3. Вы должны NOT подключиться к серверу в конструкторе. Я бы сделал это в методе start().

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

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

Чистый код Robert C. Martin также стоит прочитать.

ответил palacsint 24 42011vEurope/Moscow11bEurope/MoscowThu, 24 Nov 2011 23:50:59 +0400 2011, 23:50:59
6

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

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

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

ответил Winston Ewert 25 52011vEurope/Moscow11bEurope/MoscowFri, 25 Nov 2011 00:25:10 +0400 2011, 00:25:10

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132