Это выразительно?

Question

Следующий код является подтверждением концепции подпрограммы пакетирования сообщений.Я избегаю goto как чума и переписываю этот код?Или вы думаете, goto - это выразительный способ сделать это?

Если вы хотите переписать, напишите какой-нибудь код ...

var queue = new Queue<TraceItem>(this.batch);
while (this.connected)
{
    byte[] buffer = null;
    try
    {
        socket.Recv(out buffer);
    }
    catch
    {
        // ignore the exception we get when the socket is shut down from another thread
        // the connected flag will be set to false and we'll break the loop
    }

HaveAnotherMessage:
    if (buffer != null)
    {
        try
        {
            var item = TraceItemSerializer.FromBytes(buffer);
            if (item != null)
            {
                queue.Enqueue(item);

                buffer = null;
                if (queue.Count < this.batch && socket.Recv(out buffer, ZMQ.NOBLOCK))
                {
                    goto HaveAnotherMessage;
                }
            }
        }
        catch (Exception ex)
        {
            this.ReceiverPerformanceCounter.IncrementDiagnosticExceptions();
            this.tracer.TraceException(TraceEventType.Error, 0, ex);
        }
    }

    // queue processing code
}

Answer 1

В значительной степени подводит итог моих мыслей по поводу "goto".

Goto - плохая практика программирования по многим причинам.Главным из них является то, что у почти никогда нет причин для этого .Кто-то опубликовал цикл do..while, используйте это.Используйте boolean, чтобы проверить, следует ли продолжать.Используйте цикл while.Goto для устных языков и обратного вызова в дни ассемблера (JMP кто-нибудь?).Вы используете язык высокого уровня по причине .Чтобы вы и все остальные не смотрели на ваш код и не терялись.

---

Чтобы этот ответ был как-то актуальным, я хотел бы отметить, что комбинация goto и бодрящих ошибок вызвала серьезная ошибка SSL в iOS и OS X .

Answer 2

Замените goto на do-while или просто цикл while, если вы не хотите использовать функцию «всегда запускать один раз», которая есть у вас сейчас.

var queue = new Queue<TraceItem>(this.batch);
while (this.connected)
{
    byte[] buffer = null;
    try
    {
        socket.Recv(out buffer);
    }
    catch
    {
        // ignore the exception we get when the socket is shut down from another thread
        // the connected flag will be set to false and we'll break the loop
    }

    do {
        if (buffer != null)
        {
            try
            {
                var item = TraceItemSerializer.FromBytes(buffer);
                if (item != null)
                {
                    queue.Enqueue(item);
                    buffer = null;
                }
            }
            catch (Exception ex)
            {
                this.ReceiverPerformanceCounter.IncrementDiagnosticExceptions();
                this.tracer.TraceException(TraceEventType.Error, 0, ex);
            }
        }
    } while(queue.Count < this.batch && socket.Recv(out buffer, ZMQ.NOBLOCK))

    // queue processing code
}

Answer 3

Так удивительно легко избавиться от GOTO, что заставляет меня плакать

var queue = new Queue<TraceItem>(this.batch);
while (this.connected)
{
    byte[] buffer = null;
    try
    {
        socket.Recv(out buffer);
    }
    catch
    {
        // ignore the exception we get when the socket is shut down from another thread
        // the connected flag will be set to false and we'll break the loop
    }
    bool hasAnotherMessage = true
    while(hasAnotherMessage)
    {
        hasAnotherMessage = false;
        if (buffer != null)
        {
            try
            {
                var item = TraceItemSerializer.FromBytes(buffer);
                if (item != null)
                {
                    queue.Enqueue(item);

                    buffer = null;
                    if (queue.Count < this.batch && socket.Recv(out buffer, ZMQ.NOBLOCK))
                    {
                        hasAnotherMessage = true;
                    }
                }
            }
            catch (Exception ex)
            {
                this.ReceiverPerformanceCounter.IncrementDiagnosticExceptions();
                this.tracer.TraceException(TraceEventType.Error, 0, ex);
            }
        }
    }
    // queue processing code
}

Answer 4

Полагаю, что goto ЧИТАЕТСЯ более интуитивно понятным ... Но если вы ХОТИТЕ избежать этого, я думаю, все, что вам нужно сделать, это бросить код в цикл while(true), а затем получить оператор break в конце цикла для нормальной итерации. И goto можно заменить на оператор continue.

В конце концов вы просто учитесь читать и писать циклы и другие структуры потока управления вместо использования goto операторов, по крайней мере, по моему опыту.

Answer 5

Вид связан с постом Джоша К., но я пишу его здесь, поскольку комментарии не допускают код.

Я могу придумать вескую причину: при обходе некоторой n-мерной конструкции найти что-то.Пример для n = 3 //...

for (int i = 0; i < X; i++)
    for (int j = 0; j < Y; j++)
        for (int k = 0; k < Z; k++)
            if ( array[i][j][k] == someValue )
            {
                //DO STUFF
                goto ENDFOR; //Already found my value, let's get out
            }
ENDFOR: ;
//MORE CODE HERE...

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

Кстати, я не говорю, что мы все должны использовать gotos, но в этой конкретной ситуации я бы сделал этокак я только что упомянул.

Answer 6

Вы можете рефакторинг это что-то вроде этого.

while (queue.Count < this.batch && buffer != null)
{
    try
    {
        var item = TraceItemSerializer.FromBytes(buffer);
        buffer = null;
        if (item != null)
        {
            queue.Enqueue(item);
            socket.Recv(out buffer, ZMQ.NOBLOCK)
        }
    }
    catch (Exception ex)
    {
        this.ReceiverPerformanceCounter.IncrementDiagnosticExceptions();
        this.tracer.TraceException(TraceEventType.Error, 0, ex);
    }
}

Answer 7

Я думаю, что ваш метод слишком велик. Он смешивает различные уровни абстракции, такие как обработка ошибок, поиск сообщений и обработка сообщений.

Если вы реорганизуете его различными способами, goto естественно исчезает (примечание: я предполагаю, что ваш основной метод называется Process):

...

private byte[] buffer;
private Queue<TraceItem> queue;

public void Process() {
  queue = new Queue<TraceItem>(batch);
  while (connected) {
    ReceiveMessage();
    TryProcessMessage();
  }
}

private void ReceiveMessage() {
  try {
    socket.Recv(out buffer);
  }
  catch {
    // ignore the exception we get when the socket is shut down from another thread
    // the connected flag will be set to false and we'll break the processing
  }
}

private void TryProcessMessage() {
  try {
    ProcessMessage();
  }
  catch (Exception ex) {
    ProcessError(ex);
  }
}

private void ProcessMessage() {
  if (buffer == null) return;
  var item = TraceItemSerializer.FromBytes(buffer);
  if (item == null) return;
  queue.Enqueue(item);
  if (HasMoreData()) {
    TryProcessMessage();
  }
}

private bool HasMoreData() {
  return queue.Count < batch && socket.Recv(out buffer, ZMQ.NOBLOCK);
}

private void ProcessError(Exception ex) {
  ReceiverPerformanceCounter.IncrementDiagnosticExceptions();
  tracer.TraceException(TraceEventType.Error, 0, ex);
}

...

Answer 8

В этом случае я бы избегал перехода на goto и его рефакторинга. По моему мнению, этот метод выглядит слишком долго.

Answer 9

Обернуть «HaveAnotherMessage» в метод, который принимает в буфер и может вызывать себя рекурсивно.Казалось бы, это самый простой способ исправить это.

Answer 10

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