这个goto语句是否具有表现力？

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
}

- jsw

---

你认为 What do you think 这件事怎么样？ - Josh K

在这个问题中，“表达性”一词的翻译是：“糟糕和尴尬，但我可能可以用冗长的标签来合理化它。”请重新书写并改进它。 - Adam Crossland

至少你使用了命名标签; 我继承了一些带有数值标签的 'VB.NET' 代码。它让我回想起了 GWBasic，行号每增加 10 就为以后可能需要插入新代码而预留空间。请同情那些将来会接手你的代码的可怜的维护工程师，并避免这种诱惑。 - Dan Bryant

@Dan：我还记得在任何给定的部分中添加超过十行的恐惧。 - Josh K

除了我之外，有没有人觉得最近几天有很多与goto相关的问题被问到，这有点可疑？ - rmeador

Answer 1

这几乎总结了我对“goto”的想法。

使用goto是一种不好的编程实践，原因有很多。其中最重要的是，几乎从来没有理由使用它。有人发布了一个do..while循环，请使用它。使用一个boolean来检查是否应该继续。使用while循环。goto是为解释性语言和汇编器时代（JMP任何人？）而设计的。你使用高级语言是有原因的。这样你和其他人不会看着你的代码迷失方向。

---

为了使这个答案保持一定的时效性，我想指出，goto和括号错误的组合导致了iOS和OS X中的一个严重的SSL漏洞。

- Josh K

---

今天早些时候发生了这件事！我正在考虑编写一个小的VS2010扩展程序，当您键入goto时，它会设置计算机哔哔作响。 - jsw

实际上，GOTO语句确实有其优点，前提是只在以下情况下使用：(1)替代方案不够易读或不够直观，(2)易于跟踪程序的流程。 - user365636

例如，比较以下两种编程方式（第一种可能更易读、易懂）：-- while expression_1 while expression_2 ... if amazing_exception then goto get_out end if end while end while

get_out: ...

与下面这种替代方式（可能不如第一种方式易读、易懂）：

-- flag = false while expression_1 while expression_2 ... if amazing_exception then flag = true break end if end while if flag == true then break end if end while

许多人无缘无故地憎恨goto语句，但它确实有其用途——只是用途很少。;-) - user365636

@Thomas：我不同意：http://gist.github.com/468178。告诉我那不可读。怎么让程序流在作用域内外跳来跳去更易读了呢？ - Josh K

当然，在那个例子中，使用异常标志更易读。但是请比较一下http://gist.github.com/468890和http://gist.github.com/468893，甚至是http://gist.github.com/468894。现在，就我个人而言，我通常会选择后者---http://gist.github.com/468894---没有GOTO语句---但我确实认为GOTO 可以有其时机和地点。不要误解我的意思--我认为在大多数情况下，使用GOTO是更无效或笨拙的解决方案。但结构化编程不仅仅是减去GOTO语句的编程，正如一些人所认为的那样。 - user365636

@Thomas：即使在第一个例子中，为什么不直接在那个if语句里写error("{TEXT}"); return;呢？你的其他例子似乎并不像你知道自己在做什么时会写的东西。 - Josh K

Answer 2

使用do-while替换goto语句，或者如果您不想要当前的“始终运行一次”功能，则可以使用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
}

- corsiKa

---

我个人认为，我更喜欢Josh的答案。 - corsiKa

如果你有一个单独的 if (xxx) goto LABEL;，那么这应该是你可以将其重写为循环的强烈提示。 - Dolphin

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
}

- Randolpho

---

+1 表示几乎所有的 GOTO 语句都只需要花费一点思考时间就能避免出错。 - Adam Crossland

呵呵... 显然不是那么容易，因为我弄糟了 while 循环的主体。具有讽刺意味的是，由于 while 块中的单个 if 语句，原始代码按预期工作。 - Randolpho

Answer 4

我觉得使用goto语句可以更容易理解代码逻辑...但如果你想避免使用goto，我认为你只需要将代码放在一个while(true)循环中，然后在循环的末尾使用break语句进行常规迭代。而goto可以被替换为continue语句。

最终，你会学习如何编写循环和其他控制流结构，而不是使用goto语句，至少在我的经验中是这样的。

- Platinum Azure

Answer 5

这与Josh K的帖子有些相关，但我在此编写它，因为评论不允许代码。

我能想到一个好原因：当穿越一些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”个while循环和布尔值来判断是否应该继续...或者你可以创建一个函数，将n维数组映射到一个维度，只使用一个while循环，但我认为嵌套的for循环更易读。

顺便说一句，我并不是在说我们都应该使用goto，但在这种特定情况下，我会按照我刚才提到的方式去做。

- Carlos Muñoz

---

你可以将 i = X，j = Y，k = Z 设置好，然后使用 continue; 继续执行。 - Josh K

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);
    }
}

- ChaosPandion

Answer 7

嗯，我不太确定你是否想要跳出一个try块。虽然我不完全确定，但我相当肯定这样做是不安全的。看起来这并不是很安全...

- Michael Dorgan

---

这是否也适用于 while 循环中的 continue？（请参见我的答案） - Platinum Azure

我们知道这是有效的。我只是担心使用goto跳出关键类型部分，这似乎是一个容易出错的地方。 - Michael Dorgan

跳出作用域是安全的。 跳入作用域才是不安全的。 因此，C＃不允许您跳入作用域。 当然，如果您的关键部分在监视器命名空间或其他位置中明确处理为函数调用，则需要将其放入finally块中。 - Brian

非常公平。也很有道理。谢谢。 - Michael Dorgan

Answer 8

将"HaveAnotherMessage"包装成一个方法，该方法接受缓冲区并可以递归调用自身。这似乎是修复此问题最简单的方法。

- JB King

---

你觉得把这个方法改成递归会更好吗？:\ 简单循环有什么问题吗？ - Brendan Long

使用递归处理可能没有尽头的通过套接字传输的数据似乎不是最佳选择。 - Adam Crossland

至少现在没有人建议添加一个 try..catch(StackOverflowException) 块 :) （说明：这句话是指某个编程问题中，没有人会建议在代码中使用 try..catch 来捕获 StackOverflowException 异常，因为这种异常通常表示程序的无限递归或者栈溢出等严重问题，不应该被处理而应该被及时修复。） - Michael Stum

@Michael，我真希望我有。 - Adam Crossland

-1：递归用于无限套接字读取？那只会招来麻烦！不好的想法！ - Platinum Azure

Answer 9

我会避免在这种情况下使用goto，并进行重构。在我看来，该方法过长。

- Mikael Svenson

Answer 10

我认为你的方法太大了。它混合了不同层次的抽象，比如错误处理、消息检索和消息处理。

如果你将其重构为不同的方法，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);
}

...

- Jordão