किसी को स्विच स्टेटमेंट को खत्म करने का प्रयास कब करना चाहिए?

Question

मैं जिस कोडबेस पर काम कर रहा हूं उसमें एक स्विच स्टेटमेंट में आया हूं और मैं switch statements are considered a code smell के बाद इसे बेहतर तरीके से बदलने के तरीके को समझने की कोशिश कर रहा हूं। हालांकि, several स्टैक ओवरफ्लो पर replacingswitchstatements पर पोस्ट करके मुझे इस विशेष स्विच स्टेटमेंट को प्रतिस्थापित करने का एक प्रभावी तरीका नहीं लगता है।

इससे मुझे आश्चर्य हुआ कि यह विशेष स्विच स्टेटमेंट ठीक है और यदि कोई विशेष परिस्थितियां हैं जहां स्विच स्टेटमेंट उचित मानते हैं।

मेरे मामले कोड (थोड़ा स्वाभाविक रूप से अस्पष्ट) है कि मैं के साथ संघर्ष कर रहा हूँ में इस तरह है:

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

किसी को भी इस स्विच को खत्म करने के लिए एक तरह से बता सकते हैं? या यह एक स्विच का उचित उपयोग है? और यदि ऐसा है, तो स्विच स्टेटमेंट के लिए अन्य उचित उपयोग हैं? मैं वास्तव में जानना चाहता हूं कि वे कहां उपयुक्त हैं इसलिए मैं हर स्विच स्टेटमेंट को खत्म करने की कोशिश करने में बहुत अधिक समय बर्बाद नहीं करता क्योंकि मुझे कुछ परिस्थितियों में गंध माना जाता है।

अद्यतन:Michael के सुझाव पर मैं इस तर्क के दोहराव के लिए खोज का एक सा था और पता चला कि किसी अन्य वर्ग है कि प्रभावी ढंग से पूरे स्विच बयान बेमानी बना में तर्क बनाया था। तो कोड के इस विशेष बिट के संदर्भ में स्विच स्टेटमेंट अनावश्यक था। हालांकि, मेरा प्रश्न कोड में स्विच स्टेटमेंट की उचितता के बारे में अधिक है और क्या हमें हमेशा इस मामले में उन्हें ढूंढने की कोशिश करनी चाहिए, इस मामले में मैं इस जवाब को स्वीकार करने के इच्छुक हूं कि यह स्विच स्टेटमेंट उचित है।

Answer 1

यह एक स्विच स्टेटमेंट के लिए उपयुक्त उपयोग है, क्योंकि यह विकल्प पठनीय बनाता है, और एक को जोड़ने या घटाने में आसान बनाता है।

See this link.

Answer 2

मैं एक if का उपयोग नहीं होता। ifswitch से कम स्पष्ट होगा। 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 ठीक हो सकता है, आप @Talljoe सुझाव को देखने के लिए चाहते हो सकता है।

Answer 3

यह स्विच कथन ठीक है। क्या आपके पास भाग लेने के लिए कोई अन्य बग नहीं है? lol

हालांकि, एक बात मैंने देखी है ... आपको आईआरएडर [] ऑब्जेक्ट इंडेक्सर पर इंडेक्स ऑर्डिनल्स का उपयोग नहीं करना चाहिए .... क्या कॉलम ऑर्डर बदलते हैं? फ़ील्ड नामों का उपयोग करने का प्रयास करें यानी पाठक ["आईडी"] और पाठक ["नाम"]

Answer 4

आपके कोड में स्थित छूट प्रकार पर स्विच हैं? एक नए छूट प्रकार को जोड़ने के लिए आपको ऐसे कई स्विच संशोधित करने की आवश्यकता है? यदि ऐसा है तो आपको स्विच आउट फैक्टरिंग में देखना चाहिए। यदि नहीं, तो यहां एक स्विच का उपयोग सुरक्षित होना चाहिए।

यदि आपके कार्यक्रम के दौरान छूट विशिष्ट व्यवहार प्रसार का एक बहुत है, तो आप इस तरह refactor करने के लिए चाहते हो सकता है:

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

फिर छूट वस्तु सभी विशेषताओं और छूट पता लगाना से संबंधित तरीकों में शामिल है।

Answer 5

स्विच स्टेटमेंट (विशेष रूप से लंबे समय तक) को बुरा माना जाता है, न कि क्योंकि वे स्विच स्टेटमेंट हैं, लेकिन क्योंकि उनकी उपस्थिति रिफैक्टर की आवश्यकता का सुझाव देती है।

स्विच स्टेटमेंट्स के साथ समस्या यह है कि वे आपके कोड में एक विभाजन बनाते हैं (जैसे कि अगर कथन करता है)। प्रत्येक शाखा को व्यक्तिगत रूप से परीक्षण किया जाना चाहिए, और प्रत्येक शाखा के भीतर प्रत्येक शाखा और ... अच्छी तरह से, आपको विचार मिलता है।

कहा कि, निम्न आलेख स्विच बयान का उपयोग कर के बारे में कुछ अच्छी प्रथाओं है:

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

अपने कोड के मामले में, ऊपर के लिंक में लेख पता चलता है कि आप इस प्रदर्शन कर रहे हैं, तो एक गणना से दूसरे में रूपांतरण का प्रकार, आपको अपना स्विच अपनी विधि में रखना चाहिए, और ब्रेक स्टेटमेंट के बजाय रिटर्न स्टेटमेंट का उपयोग करना चाहिए। मैं ऐसा करने के बाद से पहले, और कोड अधिक स्वच्छ दिखता है:

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

Answer 6

मैं कोड को बदलने के लिए कोड को बदलने लगता है कि लोगों को समय का सबसे अच्छा उपयोग नहीं है। इसे बदलने के लिए कोड बदलना [अधिक पठनीय, तेज़, अधिक कुशल, इत्यादि] समझ में आता है। इसे केवल इसलिए न बदलें क्योंकि कोई कहता है कि आप कुछ 'सुगंधित' कर रहे हैं।

-रिक

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 बना रहे हैं तो इस जगह पर कई अलग-अलग जगहें जोड़ें या केवल। यदि आप कई जगहों पर एमटीपीई बना रहे हैं तो हमेशा कुछ अन्य चेक के लिए डायिक्सटाउन प्रकार के लिए स्विच करना पड़ता है तो यह कोड गंध हो सकता है।

मैं अपने प्रोग्राम में एक ही स्थान पर एमटीपीप्स के निर्माण में शायद एमटीपी के निर्माता या किसी प्रकार की फैक्ट्री विधि में निर्माण करने की कोशिश करता हूं लेकिन आपके प्रोग्राम के यादृच्छिक हिस्सों को मूल्य देने से किसी को भी पता नहीं चल सकता मूल्य कैसे होना चाहिए और कुछ गलत करना चाहिए।

तो स्विच अच्छा है, लेकिन हो सकता है स्विच अपने प्रकार

Answer 9

हाँ, यह स्विच बयान के एक सही उपयोग की तरह दिखता है के निर्माण के हिस्से के अंदर अधिक ले जाया जाना चाहिए।

हालांकि, मेरे पास आपके लिए एक और सवाल है।

आपने डिफ़ॉल्ट लेबल क्यों नहीं शामिल किया है?डिफ़ॉल्ट लेबल में अपवाद फेंकने से यह सुनिश्चित हो जाएगा कि जब आप कोई नई छूट टाइप करें और कोड को संशोधित करना भूल जाएंगे तो प्रोग्राम ठीक से विफल हो जाएगा।

इसके अलावा, यदि आप एनम के लिए एक स्ट्रिंग मान मैप करना चाहते हैं, तो आप विशेषताएँ और प्रतिबिंब का उपयोग कर सकते हैं।

कुछ की तरह:

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

जब आप एक भाषा के लिए डिजाइन और अंत में पूरी भाषा में ugliest, सबसे गैर सहज त्रुटियों की संभावना वाक्य रचना को दूर करने का मौका है।

यह तब होता है जब आप स्विच स्टेटमेंट को आजमाते हैं और हटाते हैं।

बस स्पष्ट होने के लिए, मेरा मतलब वाक्यविन्यास है। यह सी/सी ++ से लिया गया है जिसे सी # में अधिक आधुनिक वाक्यविन्यास के अनुरूप बदलने के लिए बदला जाना चाहिए था। मैं पूरी तरह से स्विच प्रदान करने की अवधारणा से सहमत हूं ताकि संकलक कूद को अनुकूलित कर सके।

Answer 11

मैं बिल्कुल कथन स्विच करने का विरोध नहीं कर रहा हूं, लेकिन यदि आप उपस्थित हैं, तो मैं कम से कम डिस्काउंट टाइप निर्दिष्ट करने के दोहराव को समाप्त कर दूंगा; मैंने इसके बजाय एक फ़ंक्शन लिखा होगा जो एक स्ट्रिंग दिए गए डिस्काउंट टाइप को देता है। उस समारोह में ब्रेक की आवश्यकता को खत्म करने, प्रत्येक मामले के लिए वापसी विवरण हो सकते थे। मुझे स्विच मामलों के बीच बहुत धोखाधड़ी के बीच ब्रेक की आवश्यकता मिलती है।

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

रॉबर्ट हार्वे और टल्जोजो ने उत्कृष्ट उत्तर प्रदान किए हैं - जो आपके पास है वह एक चरित्र कोड से एक गणित मूल्य पर मैपिंग है। यह मैपिंग के रूप में सबसे अच्छा व्यक्त किया जाता है जहां मैपिंग का विवरण एक ही स्थान पर, मानचित्र में (जैसा कि तल्जो सुझाव देता है) या एक फ़ंक्शन में होता है जो स्विच स्टेटमेंट (रॉबर्ट हार्वे द्वारा सुझाए गए अनुसार) का उपयोग करता है।

उन दोनों तकनीकों में शायद इस मामले में ठीक है, लेकिन मैं एक डिजाइन प्रिंसिपल पर आपका ध्यान आकर्षित करना चाहता हूं जो यहां या अन्य समान मामलों में उपयोगी हो सकता है। खुला/बंद प्रिंसिपल:

• http://en.wikipedia.org/wiki/Open/closed_principle
• http://www.objectmentor.com/resources/articles/ocp.pdf (! यकीन है कि आप यह पढ़ कर)

तो मानचित्रण समय के साथ बदलने के लिए, या संभवतः संभावना है बढ़ाया जा रनटाइम (उदाहरण के लिए, एक प्लगइन सिस्टम के माध्यम से या डेटाबेस से मैपिंग के हिस्सों को पढ़कर), फिर रजिस्ट्री पैटर्न का उपयोग करने से आप खुले/बंद प्रिंसिपल का पालन करने में मदद करेंगे, जिससे मैपिंग को किसी भी सह को प्रभावित किए बिना विस्तारित किया जा सकेगा। डी जो मैपिंग का उपयोग करता है (जैसा कि वे कहते हैं - एक्सटेंशन के लिए खुला, संशोधन के लिए बंद)।

मुझे लगता है कि यह रजिस्ट्री पैटर्न पर एक अच्छा लेख है - देखें कि रजिस्ट्री कुछ कुंजी से कुछ मूल्य से मैपिंग कैसे रखती है? इस तरह यह एक स्विच स्टेटमेंट के रूप में व्यक्त आपके मैपिंग के समान है।बेशक, आपके मामले में आप वस्तुओं है कि सभी एक आम इंटरफ़ेस को लागू पंजीकरण के नहीं किया जाएगा, लेकिन आप सार मिलना चाहिए: मूल सवाल का जवाब देने

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

तो, - मामले बयान खराब रूप है क्योंकि मुझे उम्मीद है कि चरित्र कोड से मैपिंग को आपके आवेदन में कई स्थानों पर एक समेकित मूल्य की आवश्यकता होगी, इसलिए इसे बाहर निकाला जाना चाहिए। मैंने जिन दो उत्तरों का संदर्भ दिया है, उन्हें आपको यह सलाह दी जाती है कि आप इसे कैसे करें। यदि, हालांकि, मैपिंग समय के साथ बदलने की संभावना है, तो इस तरह के परिवर्तन के प्रभाव से अपने कोड को इन्सुलेट करने के तरीके के रूप में रजिस्ट्री पैटर्न पर विचार करें।

Answer 13

आपको इस स्विच स्टेटमेंट पर संदेह करने का अधिकार है: किसी भी प्रकार का स्विच स्टेटमेंट जो कुछ प्रकार के आकस्मिक है, लापता बहुरूपता (या गायब सबक्लास) का संकेत हो सकता है।

हालांकि, टालजो का शब्दकोश एक अच्छा दृष्टिकोण है।

ध्यान दें कि यदि आपके enum और डेटाबेस मान तारों के बजाय पूर्णांक थे, या यदि आपके डेटाबेस मान enum नामों के समान थे, तो प्रतिबिंब काम करेगा, उदा।

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

दिया तो

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

पर्याप्त होगा।