क्या ऐसी शाखा है जो कोड गंध या अच्छी प्रैक्टिस नहीं करती है?

Question

मैं इस तरह कोड युक्त जवाब के साथ यहाँ धागे (या कम से कम commented) के लिए प्रतिक्रिया व्यक्त की है, लेकिन अगर यह एक (या अधिक) शाखाओं के कर के साथ if शाखाओं की एक श्रृंखला लिखने के लिए अच्छा है या बुरा रूप है मैं सोच रहा हूँ उनमें से कुछ भी नहीं, आम तौर पर प्रत्येक शाखा में null की जांच को खत्म करने के लिए।

एक उदाहरण (सी # कोड):

if (str == null) { /* Do nothing */ } 
else if (str == "SomeSpecialValue") 
{ 
    // ... 
} 
else if (str.Length > 1) 
{ 
    // ... 
} 

बजाय

:

if (str != null && str == "SomeSpecialValue") 
{ 
    // ... 
} 
else if (str != null && str.Length > 1) 
{ 
    // ... 
} 

और, बेशक, यह सिर्फ एक उदाहरण है, जैसा कि मैंने बड़े और अधिक जटिल के साथ इन का उपयोग करते हैं कक्षाएं। और इनमें से अधिकतर मामलों में, null मान कुछ भी करने का संकेत नहीं देगा।

मेरे लिए, यह मेरे कोड की जटिलता कम कर देता है और समझ में आता है जब मैं इसे देख। तो, क्या यह अच्छा या बुरा रूप है (एक कोड गंध, यहां तक ​​कि)?

Answer 1

यह वास्तव में निम्नलिखित से बचने के लिए अच्छा है करने के लिए इसे बदल क्योंकि यह बेकार में फिर से जाँच करता है स्थिति में से एक (तथ्य यह है कि संकलक इस दूर अनुकूलित करेंगे बिंदु के बगल में है - यह संभावित रूप से बनाता है

: अपने कोड को पढ़ने के लिए कोशिश कर रहा है लोगों) के लिए और अधिक काम 0

if (str != null && str == "SomeSpecialValue") 
{ 
    // ... 
} 
else if (str != null && str.Length > 1) 
{ 
    // ... 
} 

लेकिन यह भी कि तुम क्या सुझाव दिया, नीचे करने के लिए नहीं बल्कि विचित्र है:

if (str == null) { /* Do nothing */ } 
else if (str == "SomeSpecialValue") 
{ 
    // ... 
} 
else if (str.Length > 1) 
{ 
    // ... 
} 

मैं कहता हूँ यह विचित्र है, क्योंकि यह अपने इरादे obfuscates और खारिज कर देता है पाठक की उम्मीदों।यदि आप किसी शर्त की जांच करते हैं, तो लोग आपको संतुष्ट होने पर कुछ करने की उम्मीद करते हैं - लेकिन आप नहीं हैं। इसका कारण यह है अपने आशय नहीं वास्तव में करने के लिए प्रक्रिया अशक्त हालत, बल्कि एक नल पॉइंटर जब आप दो शर्तों आप वास्तव में में रुचि रखते हैं की जाँच से बचने के लिए है। वास्तव में, बल्कि करने के लिए दो वैचारिक राज्यों की तुलना में संभाल, एक विवेक प्रावधान (गैर अशक्त इनपुट) के साथ, इसे पढ़ता बजाय जैसे आप तीन वैचारिक राज्यों को संभालने के लिए है। यह कम स्पष्ट है - तथ्य यह है कि, computationally, आप इस तरह के तीन राज्यों बिंदु के पास है देखते हैं कह सकते हैं।

if (str != null) 
{ 
if (str == "SomeSpecialValue") 
{ 
    // ... 
} 
else if (str.Length > 1) 
{ 
    // ... 
} 
} 

इस बात से थोड़ा अधिक है: अशक्त के लिए जाँच करें, और उसके परिणाम ब्लॉक के भीतर अन्य शर्तों की जांच -

स्थिति इस तरह की में हमेशा की तरह मामले दृष्टिकोण के रूप में Oren एक सुझाव दिया है कोड गंध के मुद्दे के विपरीत, पठनीयता-बढ़ाने की शैली का।

संपादित: हालांकि, अगर आप do-कुछ भी नहीं शर्त पर सेट कर रहे हैं, मैं कर बहुत ज्यादा की तरह है कि आप एक "कुछ भी नहीं" टिप्पणी भी शामिल थे। अन्यथा, लोगों को लगता है कि आप बस कोड को पूरा करना भूल गए हैं।

Answer 2

मैं इस-

if (str != null) 
{ 
if (str == "[NULL]") 
{ 
    // ... 
} 
else if (str.Length > 1) 
{ 
    // ... 
} 
} 

की तरह यह कर पसंद करते हैं मुझे लगता है कि आप हमेशा एक खाली शरीर के साथ "reword" कर सकते हैं एक if में एक शरीर के साथ निषेध है, और यह बेहतर लग रहा है और अधिक समझ में आता है कि।

Answer 3

आपका पहला उदाहरण पूरी तरह से मेरे लिए पठनीय है - सब पर गंध नहीं करता है।

Answer 4

इस विशेष मामले में मैं जल्दी वापस आ जाएगी और यह कोड आसान

if (string.IsNullOrEmpty(str)) { return; } 

पढ़ने के लिए मैं एक स्पष्ट वापसी कथन शामिल करना चाहते बनाता है।

Answer 5

यह सब संदर्भ पर निर्भर करता। यदि खाली बयान डालने पर कोड कथन को और अधिक पठनीय बनाता है, तो उसके लिए जाएं।

Answer 6

यह पठनीय है, चाहे वह अच्छा या बुरा हो, आप जो हासिल करने की कोशिश कर रहे हैं उस पर निर्भर करता है - आम तौर पर लंबे समय से घोंसला "चला जाता है" प्रकार if कथन खराब हैं। फ्रेमवर्क में पके हुए स्थिर स्ट्रिंग विधियों के बारे में न भूलें: string.IsNullOrEmpty() और string.IsNullOrWhiteSpace()।

आपकी if (str == null) { /* Do nothing */ } लाइन असामान्य है, लेकिन इसमें एक सकारात्मक बिंदु है: यह अन्य डेवलपर्स को आगे बता रहा है कि आप जानबूझ कर उस मामले के लिए कुछ भी नहीं कर रहे हैं, यदि आपके इरादे की संरचना आपके अस्पष्ट हो सकती है ,

if (str != null) 
{ 
    /* carry on with the rest of the tests */ 
} 

Answer 7

मैं सामान्य रूप से पहले if में एक return या कुछ ऐसे ही डाल दिया जाएगा:

void Foo() 
{ 
    if (str == null) { return; } 
    if (str == "SomeSpecialValue") 
    { 
     // ... 
    } 
    else if (str.Length > 1) 
    { 
     // ... 
    } 
} 

यदि आप ऐसा नहीं कर सकते हैं, समारोह if/else के बाद कुछ और है, क्योंकि, मैं कहेंगे यह रिफैक्टर करने का समय है, और if/else भाग को एक अलग फ़ंक्शन में विभाजित करने का समय है, जिससे आप जल्दी वापस आ सकते हैं।

Answer 8

हाँ यह एक कोड गंध है।

एक संकेत यह है कि आपने इस प्रश्न को से पूछने के लिए सोचा था।

एक और संकेत है कि कोड incomplete- लगता है जैसे कुछ वहाँ से संबंधित होना चाहिए है। यह पठनीय सुनिश्चित हो सकता है, लेकिन यह महसूस करता है।

जब कि कोड को पढ़ने, एक बाहरी व्यक्ति करता है, तो कोड मान्य/पूर्ण/सही/इरादा/विशेषण के रूप में है एक पल के लिए बंद करो और निर्धारित करने के लिए brainpower का उपयोग करना पड़ता है।

मैं कहता हूँ यह विचित्र है, क्योंकि यह आपके इरादे obfuscates और पाठक की उम्मीदों खारिज कर देता है:

user359996 सिर पर कील मारा।