构建这类代码的最佳方式是什么？

我有一个函数需要分析数据包后决定要做什么。对于每个数据包代码必须：构建这类代码的最佳方式是什么？

读取数据包，超时返回错误代码。
检查是否存在腐败，如果是正面记录并且转到1.
检查中止数据包，如果是正面记录并返回并中止代码。
检查数据包参数的非法性，如果是正数登录，则返回一个无效的参数数据包并转至1.
运行数据包的操作，如果失败记录，则使用故障数据包进行响应，转到1.
如果数据包是结束数据包，则返回成功。

我的代码如下所示：

Packet p; 
for (;;) { 
    int ret = receive(&p, time); 
    if (ret == TIMEOUT) { 
     log("timeout"); 
     return TIMEOUT; 
    } 
    if (ret != 0) { 
     log("corrupted %d", ret); 
     continue; 
    } 

    if (p.type == ABORT) { 
     log("abort"); 
     return ABORT; 
    } 

    ret = check(&p); 
    if (ret != 0) { 
     log("invalid %d", ret); 
     respond(&p, INVALID); 
     continue; 
    } 

    ret = execute(&p); 
    if (ret != 0) { 
     log("failure %d", ret); 
     respond(&p, FAILURE); 
     continue; 
    } 

    if (is_last(&p)) { 
     finalize(&p); 
     return 0; 
    } 
}

是否有这个代码是没有必要的嵌套或长期更好的结构化的方式？

来源

2011-05-05 MicheleA

这绝对是个人选择，但你对我看起来不错！ – Nick 2011-05-05 07:47:42

通常，构建代码的最佳方式是以最简单的方式维护。不要选择一种对你没有意义的模式，只是因为有人告诉你它更好。 – tylerl 2011-06-05 05:42:59

我认为它看起来不错。它绝对没有不必要的嵌套，并且即使它看起来很“长”也很短 - 除非要将记录移入,check()和函数。

来源

2011-05-05 07:52:11

我会尽量避免从循环内部返回。取而代之的是在函数结束时返回一次。除此之外看起来不错。

来源

2011-05-05 07:53:31 Craig

你能解释一下为什么吗？一旦你完成了一个方法，真的没有什么有说服力的理由，一个'return'语句是一个干净的方式退出，不管它在哪里，或者有多少。 – 2011-05-05 14:40:30

如果你从多个地方返回，它会使调试变得更加困难。如果您想在返回之前添加函数调用，那么它的可扩展性也更高，因为您只需添加一次即可。 – Craig 2011-05-05 15:12:27

完全不同意它使调试更加困难，而你的第二句话是假设的（YAGNI）。如果您的架构需要它，请使用单一的'return'语句，否则这只是很多仪式，没有额外的好处。 – 2011-05-05 15:22:21

而不是环具有多return是你可以使用break和做最后的return的：

Packet p; 
int ret; 
for (;;) { 
    ret = receive(&p, time); 
    if (ret == TIMEOUT) { 
     log("timeout"); 
     break; 
    } 
    if (ret != 0) { 
     log("corrupted %d", ret); 
     continue; 
    } 

    if (p.type == ABORT) { 
     log("abort"); 
     break; 
    } 

    . 
    . 
    . 

    if (is_last(&p)) { 
     finalize(&p); 
     ret = 0; 
     break; 
    } 
} 
return ret;

来源

2011-05-05 08:11:33 Curd

通常建议在函数中只有一个'return'语句。作为一般规则，我完全不同意这条规则。通常情况下，如果函数打算返回时有明确的'return'语句，函数会更清晰，而不必遵循函数尾部的代码。另外，如果你真的必须在函数的末尾做些什么，使用两个函数对它进行建模要清楚得多，其中外部函数做了最后要做的事情，而内部使用提早返回。 – Lindydancer 2011-05-05 10:06:48

@Lindydancer：我完全同意你禁止多个'return's被高估。这就是为什么我写“你可以”而不是“你应该/必须”。然而，在这种情况下，使用'break's并不会使它变得不那么清楚，它仍然带来了单个'return'的好处，而不需要使用另一个函数（我会尽量避免）。 – Curd 2011-05-08 20:21:07

这是个人喜好，但我个人不喜欢无限循环或continue关键字。我会做这样的：

Packet p = { /* some dummy init that doesn't flag islast() true */}; 
int ret = 0; 
while (ret != TIMEOUT && p.type != ABORT && !islast(&p)) 
{ 
    int ret = receive(&p, time); 
    if (ret != TIMEOUT) 
    { 
     if (ret != 0) 
     { 
      log("corrupted %d", ret); 
     } 
     else if (p.type != ABORT) 
     { 
      ret = check(&p); 
      if (ret != 0) 
      { 
       log("invalid %d", ret); 
       respond(&p, INVALID); 
      } 
      else 
      { 
       ret = execute(&p); 
       if (ret != 0) 
       { 
        log("failure %d", ret); 
        respond(&p, FAILURE); 
       } 
      } 
     } 
    } 
} 

if (ret == TIMEOUT) 
{ 
    log("timeout"); 
} 
else if (p.type == ABORT) 
{ 
    log("abort"); 
    ret = ABORT; 
} 
else 
{ 
    finalise(&p); 
} 
return ret;

我的版本看起来比你的更复杂，但就是因为它更准确地反映了算法的结构。在我看来（这只是一个意见），关键字像继续和打破混淆结构，不应该使用。

除此之外，另一个主要优点是，在我的版本中，只需查看一个位置即环路条件，就可以清楚地看到引起环路退出的条件。此外，导致循环的条件完全在循环外部处理 - 概念上是正确的。功能也只有一个退出点。

来源

2011-05-05 08:51:19 JeremyP

我的真实代码有7个'if（cond）continue/return'，8层的嵌套太多了。 – MicheleA 2011-05-05 09:57:19

@ user586237我强烈建议不要这样的代码风格，它很难阅读，容易出错，冗余，虚假，并且需要了解Packet结构的内部知识。我老实说不会雇用提交这个的人。 – 2011-05-05 10:52:41

@ user586237：不正确，八个级别不是太多。你有八个层次的结构。因此，您应该有八层嵌套来反映这一点 - 或者您应该重构将循环内部分解为单独的功能。使用'continue'并不比删除前面的制表符来删除缩进要好。 – JeremyP 2011-05-05 10:53:18

经验法则是避免重复使用同一个变量。如果它用于新的东西，请改为创建一个新的。例如，当我读取您的代码时，我错过了ret在此过程中被重新定义的事实。另一个优点是，如果定义并立即使用一个值，则通常可以在较小的范围内定义它。

例如：如果您正在使用 ~~C99或~~ Ç

{ 
    int ret_exe = execute(&p); 
    if (ret_exe != 0) { 
     log("failure %d", ret_exe); 
     respond(&p, FAILURE); 
     continue; 
    } 
}

或者++：

ret = execute(&p); 
if (ret != 0) { 
    log("failure %d", ret); 
    respond(&p, FAILURE); 
    continue; 
}

您也可以重写此为

if (int ret_exe = execute(&p)) { 
    log("failure %d", ret_exe); 
    respond(&p, FAILURE); 
    continue; 
}

来源

2011-05-05 10:03:20 Lindydancer

'if（int ret_exe = execute（＆p））'不合法C（99或其他）。 – 2011-05-05 10:39:31

在我的经验中，这不是一个“经验法则”，我认为这不是一个特别好的方法。像'i'，'rc'，'ret'等通用临时名称的重用是常见的并且非常可读。 – 2011-05-05 10:47:12

@Jim：你当然是对的。我已经更新了答案。 – Lindydancer 2011-05-05 11:22:20

很多人不喜欢嵌套在if语句中的任务，但我认为没有任何理性的基础吨。使用它们可以实现如下紧凑的代码（我不认为它是“最好的”;这是非常主观的）：

for(;;){ 
    Packet p; 
    int ret; 

    if((ret = receive(&p, time)) == TIMEOUT){ 
     log("timeout"); 
     return TIMEOUT; 
    }else if(ret != 0){ 
     log("corrupted %d", ret); 
    }else if(p.type == ABORT){ 
     log("abort"); 
     return ABORT; 
    }else if((ret = check(&p)) != 0){ 
     log("invalid %d", ret); 
     respond(&p, INVALID); 
    }else if((ret = execute(&p)) != 0){ 
     log("failure %d", ret); 
     respond(&p, FAILURE); 
    }else if(is_last(&p)){ 
     finalize(&p); 
     return 0; 
    } 
}

来源

2011-05-05 10:43:02

构建这类代码的最佳方式是什么？

回答

相关问题