什麼時候應該嘗試消除switch語句？

Question

我在我正在處理的代碼庫中遇到了一個switch語句，我試圖弄清楚如何從switch statements are considered a code smell更好地替換它。然而，通過several在flipoverflow上關於replacingswitchstatements的帖子我似乎無法想到一個有效的方式來取代這個特定的開關語句。

它讓我想知道這個特定的switch語句是否正常，以及是否存在switch語句被認爲合適的特定情況。

在我的情況下我與掙扎的代碼（自然略有混淆）是這樣的：

private MyType DoSomething(IDataRecord reader) 
{ 
    var p = new MyType 
       { 
        Id = (int)reader[idIndex], 
        Name = (string)reader[nameIndex] 
       } 

    switch ((string) reader[discountTypeIndex]) 
    { 
     case "A": 
      p.DiscountType = DiscountType.Discountable; 
      break; 
     case "B": 
      p.DiscountType = DiscountType.Loss; 
      break; 
     case "O": 
      p.DiscountType = DiscountType.Other; 
      break; 
    } 

    return p; 
} 

任何人都可以提出一個方法來消除這種開關？或者這是一個適當的使用開關？如果是的話，switch語句還有其他適當的用法嗎？我真的很想知道它們在哪裏是合適的，所以我不會浪費太多時間來試圖消除我遇到的每個開關語句，只是因爲在某些情況下它們被認爲是嗅覺。

更新：在的Michael建議我做了一些搜索出該邏輯重複的，發現有人在另一個類中，有效地使整個switch語句創建冗餘的邏輯。所以在這段代碼中，switch語句是不必要的。然而，我的問題更多的是關於switch語句在代碼中的適用性，以及我們是否應該總是試圖在發現它們時進行替換，因此在這種情況下，我傾向於接受這個switch語句合適的答案。

Answer 1

這是一個適用於開關狀態的選項，因爲它使選項可讀，並且易於添加或減少選項。

See this link.

Answer 2

我不會用一個if。 if將不如switch明確。 switch告訴我你在比較同一件事。

只是爲了嚇唬人，這是不是你的代碼不太清楚：

if (string) reader[discountTypeIndex]) == "A") 
    p.DiscountType = DiscountType.Discountable; 
else if (string) reader[discountTypeIndex]) == "B") 
    p.DiscountType = DiscountType.Loss; 
else if (string) reader[discountTypeIndex]) == "O") 
    p.DiscountType = DiscountType.Other; 

這switch可能是OK，你可能想看看@Talljoe建議。

Answer 3

這個switch語句很好。你們沒有任何其他的錯誤嗎？大聲笑

但是，有一件事我注意到了......你不應該在IReader []對象索引器上使用索引序號......如果列順序改變怎麼辦？嘗試使用字段名即閱讀器[「id」]和閱讀器[「名稱」]

Answer 4

開關是否位於整個代碼中的折扣類型？添加新的折扣類型是否需要您修改幾個這樣的交換機？如果是這樣的話，你應該考慮把交換出去。如果沒有，在這裏使用開關應該是安全的。

如果有很多折扣特定行爲傳遍你的程序，你可能要重構這個樣：

p.Discount = DiscountFactory.Create(reader[discountTypeIndex]); 

然後貼現對象包含所有相關搞清楚折扣的屬性和方法。

Answer 5

開關語句（尤其是長的）被認爲是不好的，不是因爲它們是switch語句，而是因爲它們的存在表明需要重構。

switch語句的問題是它們在代碼中創建了一個分叉（就像if語句一樣）。每個分支必須單獨進行測試，並且每個分支內的每個分支......以及您的想法。

這就是說，下面的文章對使用switch語句的一些好的做法：

http://elegantcode.com/2009/01/10/refactoring-a-switch-statement/

在你的代碼的情況下，在上面的鏈接的文章表明，如果你執行這個從一個枚舉到另一個枚舉的轉換類型，您應該將自己的開關放在自己的方法中，並使用return語句而不是break語句。我這樣做過，並且代碼看起來更清潔：

private DiscountType GetDiscountType(string discount) 
{ 
    switch (discount) 
    { 
     case "A": return DiscountType.Discountable; 
     case "B": return DiscountType.Loss; 
     case "O": return DiscountType.Other; 
    } 
} 

Answer 6

我認爲改變代碼的緣故更改代碼不是那些時間充分利用。改變代碼使其[更可讀，更快，更高效等等]是有道理的。不要僅僅因爲有人說你在做一些「臭」的事情而改變它。

-Rick

Answer 7

在我看來，這不是將那些氣味語句​​，它的裏面有什麼他們。這個開關語句對我來說沒問題，直到它開始添加更多的情況。那麼它可能是值得創建一個查找表：

private static Dictionary<string, DiscountType> DiscountTypeLookup = 
    new Dictionary<string, DiscountType>(StringComparer.Ordinal) 
    { 
     {"A", DiscountType.Discountable}, 
     {"B", DiscountType.Loss}, 
     {"O", DiscountType.Other}, 
    }; 

根據您的觀點，這可能是或多或少可讀。

事情開始變臭的地方是你的案件內容超過一兩行。

Answer 8

我認爲這取決於如果你正在創建MType添加許多不同的地方或只在這個地方。如果你在許多地方創建MType，總是必須切換爲dicsount類型的其他檢查，那麼這可能是一種代碼異味。

我會嘗試在你的程序中的一個地方獲得MTypes的創建也許在MType本身的構造函數或某種工廠方法，但有隨機部分的程序分配值可能會導致某人不知道價值觀應該如何，做錯了什麼。

所以開關是好的，但也許切換需要移動更多的類型

Answer 9

是的，這看起來像switch語句的正確用法的創建部分內。

但是，我有另一個問題給你。

爲什麼你不包含默認標籤？在默認標籤中投擲異常將確保當您添加新的discountTypeIndex並忘記修改代碼時，程序將正常失敗。

此外，如果您想將字符串值映射到Enum，則可以使用Attributes和reflection。

喜歡的東西：

public enum DiscountType 
{ 
    None, 

    [Description("A")] 
    Discountable, 

    [Description("B")] 
    Loss, 

    [Description("O")] 
    Other 
} 

public GetDiscountType(string discountTypeIndex) 
{ 
    foreach(DiscountType type in Enum.GetValues(typeof(DiscountType)) 
    { 
     //Implementing GetDescription should be easy. Search on Google. 
     if(string.compare(discountTypeIndex, GetDescription(type))==0) 
      return type; 
    } 

    throw new ArgumentException("DiscountTypeIndex " + discountTypeIndex + " is not valid."); 
} 

Answer 10

當您設計語言，終於有機會以消除整個語言中最醜陋，最不直觀容易出錯的語法。

這是當您嘗試並刪除switch語句。

只是要清楚，我的意思是語法。這是從C/C++中取得的，它應該被改變以符合C＃中更現代的語法。我完全同意提供開關的概念，以便編譯器可以優化跳轉。

Answer 11

我並不完全反對switch語句，但在你提出的情況下，我至少已經消除了指定DiscountType的重複;我可能會寫一個函數返回給定字符串的DiscountType。該功能可以簡單地爲每個案例返回陳述，而不需要休息。我發現切換案件之間的中斷需要非常危險。

private MyType DoSomething(IDataRecord reader) 
{ 
    var p = new MyType 
       { 
        Id = (int)reader[idIndex], 
        Name = (string)reader[nameIndex] 
       } 

    p.DiscountType = FindDiscountType(reader[discountTypeIndex]); 

    return p; 
} 

private DiscountType FindDiscountType (string key) { 
    switch ((string) reader[discountTypeIndex]) 
    { 
     case "A": 
      return DiscountType.Discountable; 
     case "B": 
      return DiscountType.Loss; 
     case "O": 
      return DiscountType.Other; 
    } 
    // handle the default case as appropriate 
} 

很快，我已經注意到FindDiscountType（）真的屬於DiscountType類並移動了函數。

Answer 12

羅伯特哈維和Talljoe提供了很好的答案 - 你在這裏是一個從字符代碼到枚舉值的映射。這最好表示爲一個映射，其中映射的細節在一個地方提供，無論是在地圖中（如Talljoe所建議的），還是在使用switch語句的函數中（如Robert Harvey所建議的）。

這兩方面的技術是在這種情況下可能很好，但我想提醒你注意一個設計原則可能是有用的在這裏或在其他類似的情況。 的打開/關閉本金：

• http://en.wikipedia.org/wiki/Open/closed_principle
• http://www.objectmentor.com/resources/articles/ocp.pdf（！請務必閱讀本）

如果映射可能會改變隨着時間的推移，或可能延長運行時（例如，通過插件系統或通過讀取數據庫中的映射部分），那麼使用註冊表模式將幫助您遵守開放/關閉主體，實際上允許擴展映射而不影響任何公司de使用映射（正如他們所說 - 打開擴展，關閉修改）。

我認爲這是關於註冊表模式的一篇很好的文章 - 看看註冊表如何保存從某個鍵到某個值的映射？用這種方式，它與您的映射表示爲switch語句相似。當然，你的情況，你將不會被註冊，所有實現一個共同的接口的對象，但你應該得到的要點：

• http://sinnema313.wordpress.com/2009/03/01/the-registry-pattern/

因此，要回答原來的問題 - case語句是不好的形式，因爲我預計從應用程序的多個位置需要從字符代碼到枚舉值的映射，因此應該將其分解出來。我提到的兩個答案爲您提供瞭如何做到這一點的好建議 - 請選擇您喜歡的方式。但是，如果映射可能隨時間而改變，請考慮使用註冊表模式作爲隔離代碼免受此類更改影響的一種方式。

Answer 13

你有權懷疑這個switch語句：任何switch語句取決於某種類型的東西可能表示缺少多態性（或缺少子類）。

但是，TallJoe的字典是一個很好的方法。

請注意，如果您的枚舉和數據庫值是整數而不是字符串，或者如果您的數據庫值與枚舉名稱相同，則反射會起作用。鑑於

public enum DiscountType : int 
{ 
    Unknown = 0, 
    Discountable = 1, 
    Loss = 2, 
    Other = 3 
} 

然後

p.DiscountType = Enum.Parse(typeof(DiscountType), 
    (string)reader[discountTypeIndex])); 

就足夠了。