構建這類代碼的最佳方式是什麼？

我有一個函數需要分析數據包後決定要做什麼。對於每個數據包代碼必須：構建這類代碼的最佳方式是什麼？

讀取數據包，超時返回錯誤代碼。
檢查是否存在腐敗，如果是正面記錄並且轉到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

構建這類代碼的最佳方式是什麼？

回答

相關問題