Beispiele für nutzlos/Junk-Code und einfache Korrekturen

Question

Ich kam vor kurzem auf einigen Code eingereicht mit dem folgenden

} catch (FooException e) { 
//do something 
} catch (BarException e) { 
//do something 
} catch (Exception e) { 
    throw e; 
} 

die als

leicht neu geschrieben wird

} catch (FooException e) { 
//do something 
} catch (BarException e) { 
//do something 
} 

und

if (!(flag == false)) 

, die leicht umgeschrieben wird als

if (flag) 

Was sind einige der häufigsten Junk-Code-Beispiele und der einfachere/klarere/weniger ausführliche Ersatzcode.

Answer 1

if (x == 1) 
    return true; 
else 
    return false; 

Rewrite:

return (x == 1); 

Answer 2

http://thedailywtf.com/ hat viele Beispiele für schlechten Code wie folgt.

Answer 3

Ihre Beispiele verschiedene Dinge tun.

Verwirft die ursprüngliche Ausnahme, erstellt eine neue Ausnahme im Kontext der aktuell ausgeführten Methode und löst diese aus. Die ursprüngliche Ausnahme wird verloren gehen. Das ist selten das, was Sie tun möchten, Ihr neu geschriebenes Beispiel ist besser.

Answer 4

Werfen Sie einen Blick auf alle Regeln, die statische Code-Analyse-Tools wie PMD und Findbugs verwenden.

Answer 5

ternärer Operator

dim str 
if(predicate) 
    str = "a" 
else 
    str = "b" 
end 

Rewrite

String str = iff(predicate, "a", "b") 

Answer 6

Initialisierung mit dem Wert, wenn im Fall eines Null-Standard stattdessen verwendet wird.

Foo x = null;  
if(something != null) { 
    x = something; 
} else { 
    x = default; 
} 

=>

Foo x = something != null ? something : default; 

In Sprachen, in denen wenn ein experssion:

Foo x = if(something != null) something else default; 

Answer 7

(bColour)?bColour=false:bColour=true; //could have done (i%2!=0)?bColour=true:bColour=false; 

statt:

bColour = !bColour; //could have done bColour=(i%2!=0); 

Ja, der Kommentar zum ursprünglichen Chaos war tatsächlich im Code, also korrigierte ich es auch.

Answer 8

C-String Manipulation im alten Stil scheint für viele Leute ein Bugaboo zu sein.

Ich habe zwei hier in meinem Emacs-Puffer.Beispiel 1:

strcpy(label, "\0"); 
strcpy(comm1, "\0"); //!!!20060818 Added protective code for missing nulls 
strcpy(comm2, "\0"); 
strcpy(dir, "\0"); 

Dies ist im Grunde eine Routine mit allen dazugehörigen Kopf ruft genau das zu tun:

label[0] = '\0'; 

Hier ist eine noch Phantasie Variante später im Code:

strncpy(inq.szComment, "\0", 1); 

Auch dies ist nur eine sehr aufwendige und langsame Art, dem ersten Element von inq.szComment das Nullzeichen '\ 0' zuzuweisen.

Beispiel 2:

// This is always good for string operations, but may not be necessary. 
strcat(RpRecPrefix, "\0"); 
strcat(RpCircPrefix, "\0"); 
strcat(RpEventPrefix, "\0"); 
strcat(RpFdk, "\0"); 
strcat(RpAic, "\0"); 
strcat(RpDcl, "\0"); 
strcat(RpVis, "\0"); 
strcat(RpSnd, "\0"); 
strcat(RpL3Fsi, "\0"); 
strcat(RpStartState, "\0"); 
strcat(RpContRun, "\0"); 

Diese finden die Nullen am Ende der ersten String-Parameter, und tun dann nichts. Ich kann nicht sagen, ob der Autor dachte, er würde sich davor schützen, keine abschließende Nul zu haben, oder wenn er dachte, dass das am Ende eine extra Null würde. Wie auch immer, er hat sich geirrt.

OK. Hier ist mein persönlicher Favorit.

if (*pnFrequency == 1) { 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at 1 Hz", 
       *pliWriteRate); 
    } 
    else if (*pnFrequency == 2) { 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at 2 Hz", 
       *pliWriteRate); 
    } 
    else if (*pnFrequency == 4) { 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at 4 Hz", 
       *pliWriteRate); 
    } 
    else if (*pnFrequency == 10) { 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at 10 Hz", 
       *pliWriteRate); 
    } 
    else if (*pnFrequency == 20) { 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at 20 Hz", 
       *pliWriteRate); 
    } 
    else if (*pnFrequency == 40) { 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at 40 Hz", 
       *pliWriteRate); 
    } 
    else if (*pnFrequency == 60) { 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at 60 Hz", 
       *pliWriteRate); 
    } 
    else { // above frequencies used for analysis 
     sprintf(szLogMsg, "Write to disk speed = %d bytes/sec", *pliWriteRate); 
    } 

beachte, dass der Wert von * pnFrequency wir in jedem Zweig überprüfen, ist identisch mit dem Wert hart codiert in seine sprintf, und das ist der einzige Unterschied in jedem und jeden Zweig. Das Ganze hätte sein müssen:

sprintf(szLogMsg, "Write to disk speed = %d bytes/sec at %d Hz", *pliWriteRate, 
     *pnFrequency); 

ich einfach nur nicht vorstellen, wie diese Datei 12.000 Zeilen lang ...

Answer 9

List<String> list = new ArrayList<String>(); 
list.add("Foo"); 
list.add("Bar"); 

=>

List<String> list = Arrays.asList("Foo", "Bar"); 

sein bekam In welche Fallliste wäre unveränderlich. Wenn Sie das gleiche mit einem wandelbaren Liste erreichen wollen, erstellen Sie eine util Klasse:

public CollectionUtils { 
    public static List<T> createMutableList(T ... elements) { 
    List<T> list = new ArrayList<T>(); 

    for(T elem : elements) list.add(elem); 

    return list; 
    } 
} 

Und dann können Sie es verwenden:

import static CollectionUtils.createMutableList;  
List<String> list = createMutableList("Foo", "Bar"); 

Answer 10

Vor Linq:

for each listItem1 in list1 
    for each listItem2 in list2 
    if listItem1.value1 = listItem2.value2 
    listSimilarItems.add(listItem1) 
    end 
end 

Nach Linq :

listSimilarItems = (from items in list1 where list1.value1 = list2.value2).tolist 

Answer 11

Auf der Web-Seite von p rogramming, etwas, das ich oft sehen, ist die Verwendung von CSS-Regeln wie folgt aus:

<div class="red">Error message!</div> 

Mit dem red Stil definiert als:

red { color: #f00; } 

Nun, das ist dumm.Und dann, wenn sie entscheiden, dass die Fehlermeldung auch einen gelben Hintergrund Sie das sehen soll:

<div class="red yellowback">Error message 2.0!</div> 

Oder noch schlimmer:

<div class="yellowback"> 
    <h1 class="red">Error message 3.0!</h1> 
</div> 

Der richtige Weg, CSS zu verwenden, ist Ihren HTML in einem konstruieren semantisch korrekten Weg zuerst, und identifizieren Sie dann jedes Teil mit entweder 'id' oder 'class' Namen, so dass Sie jeden Abschnitt ausrichten und die entsprechenden Regeln anwenden können. Zum Beispiel:

<div id="errormessages">Error message!</div> 

Statt ein <div> Sie es einen Absatz machen könnten, so dass es die auffallen, auch ohne Stile (zB bei Verwendung eines Text-Browser):

<p id="errormessages">Error message!</p> 

Und die CSS-Regel :

#errormessages { 
    color: #f00; 
    background-color: #ff0; 
}