Angepinnt Beispiele für guten und schlechten Code (Stil)

  • VB.NET

Es gibt 269 Antworten in diesem Thema. Der letzte Beitrag () ist von Hinti.

    Beispiele für guten und schlechten Code (Stil)

    Hallo,

    ich werde in diesem Beitrag versuchen eine Idee von gutem Code (Stil) zu vermitteln.
    Dafür werde ich das ganze - immer wenn es einen Anlass dazu gibt - erweitern.

    Das ganze ist natürlich, wie die meisten Dinge subjektiv.
    Dennoch glaube ich, einigen von euch einen Anstoss geben zu können.

    Da ich aktuelle Codes aus dem Forum verwenden werde, bitte ich euch - falls Ihr einen eurer Codes findet - das nicht persönlich zu nehmen.

    1. Verwende möglichst keine festen Zahlen, wenn es dafür eine Konstante gibt:
    In vielen Fällen gibt es im Framework Konstanten (Enums) deren Text mehr aussagt als die nackte Zahl, die sich dahinter verbirgt.

    Beispiel:

    VB.NET-Quellcode

    1. If GetAsyncKeyState(13) Then
    2. ....
    3. End If

    Der informierte weiss sicherlich, dass die 13 hier ein Enter Symbolisiert.
    Schöner, weil besser lesbar ist aber folgendes

    VB.NET-Quellcode

    1. If GetAsyncKeyState(Keys.Return) Then
    2. ....
    3. End If


    2. Suche von Zeichenfolgen in einem String:
    Oft verwenden die Leute Schleifen, wenn ein Teilstring in einem String gesucht werden soll.

    Beispiel:

    VB.NET-Quellcode

    1. For x= 1 To Len(txt)
    2. If Mid(String,"suchlänge")="suchbegriff" Then
    3. 'auszuführennde befehe
    4. End If
    5. Next x

    Hier ist eigentlich alles Furchtbar.
    1. Der Code stammt mit ziemlicher Sicherheit aus VB6
    2. Es wurde eine Schleife zur Suche von "suchbegriff" verwendet.

    So ist es übersichtlicher und kürzer.

    VB.NET-Quellcode

    1. If txt.Contains("was auch immer") = True Then
    2. 'ausführende befehle
    3. End If


    Vielleicht noch eine kleine Ergänzung.
    Wer die Position eines Teilstrings sucht, macht das am besten so.

    VB.NET-Quellcode

    1. Dim pos As Integer = txt.IndexOf("was auch immer")


    3. Beachte immer Deine Datentypen und vermische sie nicht:
    Auch wenn es mit den Standardeinstellungen keinen Fehler verursacht, sollten Datentypen nicht wild gemischt werden.
    Wenn mal eine Umwandlung von einem in den anderen Datentyp nötig ist, gibt es dafür eigene Funktionen.

    Beispiel:

    VB.NET-Quellcode

    1. Dim pos As Integer = Me.ListBox1.IndexFromPoint(e.Location)
    3. If pos = "-1" Then
    4. ...
    5. End If

    Die Variable pos ist hier ganz eindeutig und klar als Integer definiert worden - was auch richtig ist.
    Verglichen wird pos aber mit einem String.
    Dies verursacht zwar keinen Fehler, sollte jedoch vermieden werden.

    So sollte der Beispielcode aussehen.

    VB.NET-Quellcode

    1. Dim pos As Integer = Me.ListBox1.IndexFromPoint(e.Location)
    3. If pos = -1 Then
    4. ...
    5. End If


    4. Verwende Try...Catch Blöcke nur dort, wo unerwartete Fehler auftreten können:
    Try ... Catch Blöcke wurden erfunden um unerwartet auftretende Fehler abfangen zu können.
    Es macht wenig Sinn Code in Try ... Catch Blöcke zu packen, bei denen nichts unerwartetes Auftreten kann.

    Beipiel:

    VB.NET-Quellcode

    1. Try
    2. If move = True Then
    3. ListBox1.Items.Remove(eintrag)
    4. ListBox1.Items.Insert(pos, eintrag)
    5. move = False
    6. ListBox1.SelectedIndex = pos
    7. End If
    9. Catch
    11. End Try

    Mit diesem Codeschnippsel sollte demonstriert werden wie Einträge einer Listbox mit der Maus verschoben werden können.
    Die Listbox ist in diesem Fall in sich geschlossen und unerwartete Situationen sind nicht zu erwarten.
    Unerwartet wäre z.B. eine evtl. Zugriffsverletzung in Verbindung mit Dateien.

    Der obige Code kann also genausogut ohne den Try ... Catch Block geschrieben werden.

    VB.NET-Quellcode

    1. If move = True Then
    2. ListBox1.Items.Remove(eintrag)
    3. ListBox1.Items.Insert(pos, eintrag)
    4. move = False
    5. ListBox1.SelectedIndex = pos
    6. End If


    5. Verweden von Select Case Blöcken anstatt vieler If Anweisungen:
    Oft ist es Sinnvoll Select Case Blöcke anstatt eine ganze Reihe von If Anweisungen zu verwenden.
    Dies hat den Vorteil, das es ohne großen aufwand und ohne die Übersichtlichkeit des Codes zu gefährden erweitert werden kann.

    Klar ist natürlich auch, dass nicht immer ein Select Case Block verwendet werden kann.
    Im folgenden Beispiel jedoch schon.

    VB.NET-Quellcode

    1. If SFD.FilterIndex = 0 Then
    2. SsHot.Save(SFD.FileName, System.Drawing.Imaging.ImageFormat.Png)
    3. Exit Sub
    4. End If
    6. If SFD.FilterIndex = 1 Then
    7. SsHot.Save(SFD.FileName, System.Drawing.Imaging.ImageFormat.Bmp)
    8. Exit Sub
    9. End If
    11. If SFD.FilterIndex = 2 Then
    12. SsHot.Save(SFD.FileName, System.Drawing.Imaging.ImageFormat.Gif)
    13. Exit Sub
    14. End If

    Dadurch, dass Filterindex immer nur einen Wert annehmen kann - drängt sich hier Select Case einfach auf.
    Die mögliche Erweiterbarkeit - z.B. wenn es einen neuen Wert abzufragen gibt - spricht auch dafür.

    So würde es als Select Case Block aussehen.

    VB.NET-Quellcode

    1. Select Case SFD.FilterIndex
    2. Case 0
    3. SsHot.Save(SFD.FileName, System.Drawing.Imaging.ImageFormat.Png)
    4. Case 1
    5. SsHot.Save(SFD.FileName, System.Drawing.Imaging.ImageFormat.Bmp)
    6. Case 2
    7. SsHot.Save(SFD.FileName, System.Drawing.Imaging.ImageFormat.Gif)
    8. Case Else
    9. 'Case Else sollte immer Programmiert werden, um auf ungewünschte
    10. 'bzw. unerwartete Ereignisse aufmekrsam gemacht werden zu können
    11. End Select


    6. Das Using Statement verwenden:
    Werden verwendete Klassen nicht mehr gebraucht, sollten diese mit Dispose wieder freigegeben werden,
    dies gilt vor allem für nicht verwaltete Resourcen. (z.B COM Objekten oder SQL Verbindungen)
    (Damit später der Garbage Collector seine Arbeit tun kann.)
    Durch Verwenden des Using Statements braucht man sich um solche Dinge nicht mehr zu kümmern.
    Der Kompiler erledigt alle nötigen Aufrufe.

    Beipsiel:

    VB.NET-Quellcode

    1. Dim SFD As New SaveFileDialog
    2. SFD.Filter = "PNG-Bilder(*.PNG)|*.png|BMP-Bilder(*.bmp)|*.BMP|GIF-Bilder(*.GIF)|*.gif"
    3. If SFD.ShowDialog = Windows.Forms.DialogResult.OK Then
    4. ...
    5. End If

    Bei verwaltetem Code wie z.b. dem SaveFileDialog hat Using den Vorteil, eine gewisse Übersicht in den Quellcode zu bringen.
    Übersichtlicher Code ist besser lesbar, was natürlich sehr wichtig ist.
    Deshalb ist es Ratsam auch hier den Using Block zu verwenden.

    VB.NET-Quellcode

    1. Using SFD As New SaveFileDialog
    2. SFD.Filter = "PNG-Bilder(*.PNG)|*.png|BMP-Bilder(*.bmp)|*.BMP|GIF-Bilder(*.GIF)|*.gif"
    3. If SFD.ShowDialog = Windows.Forms.DialogResult.OK Then
    4. ...
    5. End If
    6. End Using


    7. Verwende nie bekannte Eigenschaftsnamen als Variablenname:
    Visual Basic erlaubt es bekannte Eigenschaftsnamen auch als Variable zu verwenden.
    Dies macht den Code äußerst schlecht lesbar.
    Jeder verbindet mit Eigenschaften wie z.b. "Text" eine Eigenschaft und keine Variable
    Beispiel:

    VB.NET-Quellcode

    1. Dim Text As System.Text.StringBuilder = New System.Text.StringBuilder(TextBox1.Text)
    4. For i = 0 To XXX 'For-Schleife
    5. TextBox1.Text = Text.Remove(0, 3).ToString 'Löscht die ersten 3 Zeichen
    6. Next

    was spräche dagegen - hier einen anderen Variablennamen zu verwenden.
    Einfach der Übersicht halber.

    VB.NET-Quellcode

    1. Dim sb As System.Text.StringBuilder = New System.Text.StringBuilder(TextBox1.Text)
    4. For i = 0 To XXX 'For-Schleife
    5. TextBox1.Text = sb.Remove(0, 3).ToString 'Löscht die ersten 3 Zeichen
    6. Next

    sieht doch schon viel klarer aus - oder ?

    Gruss

    mikeb69

    Dieser Beitrag wurde bereits 7 mal editiert, zuletzt von „mikeb69“ ()

    Ebenfalls grausig finde ich tiefe IF-Verschachtelung:

    VB.NET-Quellcode

    1. If 1 = 1 Then
    2. If 2 = 2 Then
    3. If 3 = 3 Then
    4. ...
    5. End If
    6. End If
    7. End If


    Viel besser finde ich folgendes:

    VB.NET-Quellcode

    1. Select Case True
    2. Case 1 = 1
    3. Case 2 = 2
    4. Case 3 = 3
    5. End Select

    Ich teile die Ansicht von vanitas-mundi. Jedoch: Wenn schon Code-Beispiele, dann bitte so:

    Statt diesem hier:

    VB.NET-Quellcode

    1. If x = 1 Then
    2. '...
    3. ElseIf x = 2 Then
    4. '...
    5. ElseIf x = 3 Then
    6. '...
    7. End If


    Das hier:

    VB.NET-Quellcode

    1. Select Case x
    2. Case 1: '...
    3. Case 2: '...
    4. Case 3: '...
    5. End Select

    @ mikeb69:

    Es kann hier durchaus zu einem Fehler kommen:

    Quellcode

    1. If move = True Then
    2. ListBox1.Items.Remove(eintrag)
    3. ListBox1.Items.Insert(pos, eintrag)
    4. move = False
    5. ListBox1.SelectedIndex = pos
    6. End If



    Zum Beispiel wenn nichts gewählt ist und er versucht, einen leeren Eintrag zu löschen oder wenn die Maustaste an einer Stelle in der Listbox losgelassen
    wird, wo nichts ist (also auch keine leeren Einträge), dann versucht er nämlich das Item an Position -1 einzufügen.

    Unerwartete Fehler können zudem fast immer auftreten - sogar beim laden einer Textdatei, die zum Beispiel geschützt ist. Daher verwende ich den Try-Catch Block auch an Stellen, an denen man nicht einen Fehler erwartet - User hassen Fehler.

    Schlechter Stil ist meiner Meinung nach die Verwendung von Try-Catch, wenn man weiß, dass ein Fehler auftritt.

    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „kevin89“ ()

    Schlechter Stil ist meiner Meinung nach die Verwendung von Try-Catch, wenn man weiß, dass ein Fehler auftritt.


    Ich hoffe ich habe dich nicht falsch verstanden, aber da muss ich entschieden intervenieren :)

    Nimm mal dieses Beispile hier:
    Ich "weiss" was für Fehler auftreten (können) finde aber dennoch die Lösung mit try und catch schöner, einfacher, schneller, sauberer:

    VB.NET-Quellcode

    1. Dim element As String = "PfadZurDateiAusVorherigenFunktionen"
    3. Try
    4. My.Computer.FileSystem.DeleteFile(element)
    5. Catch ex As System.IO.FileNotFoundException
    6. MsgBox("Datei nicht gefunden")
    7. Catch ex As ArgumentNullException
    8. MsgBox("Kein Pfad übergeben")
    9. Catch ex As AccessViolationException
    10. MsgBox("Keine Rechte")
    11. Catch ex As UnauthorizedAccessException
    12. MsgBox("Datei gerade in Verwendung")
    13. End Try


    Ohne Try Catch wäre da wohl sowas hier nötig, wobei die ganzen funktionen die ich "mal so genannt habe" natürlich auch noch programmiert werden müssten:

    VB.NET-Quellcode

    1. Dim element As String = "PfadZurDateiAusVorherigenFunktionen"
    2. If element IsNot Nothing Then
    3. If existiertDatei(element) Then
    4. If habIchRechteZumLöschen(element) Then
    5. If istloeschbar(element) Then
    6. loscheDatei(element)
    7. Else
    8. MsgBox("Datei gerade in Verwendung")
    9. End If
    10. Else
    11. MsgBox("Keine Rechte")
    12. End If
    13. Else
    14. MsgBox("Datei nicht gefunden")
    15. End If
    16. Else
    17. MsgBox("Kein Pfad übergeben")
    18. End If


    grüße,
    dognose

    Auch finde ich es gräßlich, Rückgabewerte vom Typ Boolean folgendermaßen zu übergeben: (Es sei nun einmal dahingestellt, ob diese Funktion sinnvoll ist. Falls es ein komplexerer Vergleich ist, der oft gebraucht wird, ist es sinnvoll, diesen in eine Funktion auszulagern, damit der eigentliche Code besser lesbar bleibt)

    VB.NET-Quellcode

    1. Private Function xyz() As Boolean
    2. If (1 = 1) Then
    3. Return True
    4. Else
    5. Return False
    6. End If
    7. End Function

    Dageben ist folgende Möglichkeit wesentlich kürzer und auch besser:

    VB.NET-Quellcode

    1. Private Function xyz() As Boolean
    2. Return (1 = 1)
    3. End Function

    Auch muss man nicht immer schreiben

    VB.NET-Quellcode

    1. If xyz = True

    sondern kann einfach

    VB.NET-Quellcode

    1. If xyz
    benutzen. In anderen Programmiersprachen wird das fast immer so gemacht.



    Quellcode

    1. If move = True Then
    2. ListBox1.Items.Remove(eintrag)
    3. ListBox1.Items.Insert(pos, eintrag)
    4. move = False
    5. ListBox1.SelectedIndex = pos
    6. End If




    ich überlese einfach mal :

    VB.NET-Quellcode

    1. if move = True then

    grausamere fehler gibts kaum :p

    Genau. Bei If kein True oder False reinbringen, sondern kein Boolscher Operand und bei Bedarf einfach die Bedingung mit ! umkehren.
    Was ich auch grässlich finde, ist es, wenn man statt MessageBox.Show() einfach nur MsgBox nimmt.
    Und am schlimmsten ist es, dass VB automatisch implizit konvertiert. Also folgendermaßen:

    VB.NET-Quellcode

    1. Dim s As Integer
    2. s = "1234"

    Was natürlich falsch ist. In C# gäbe das Äquivalent einen Compilerfehler. Also müsste man hier schreiben (angenommen der String "1234" kommt von woanders her):

    VB.NET-Quellcode

    1. Dim s As Integer
    2. s = Convert.ToInt32("1234")

    Variablen verwenden

    Statt x mal etwas immer wieder einzugeben, wie hier:

    VB.NET-Quellcode

    1. If File.Exists("C:\test.txt") Then
    2. File.delete("C:\text.txt")
    3. Else
    4. 'Irgendwas
    5. End If


    sollte man lieber eine Variable benutzen, die man nur 1 mal ändern muss.



    VB.NET-Quellcode

    1. Dim strDatei As String = "C:\test.txt"
    3. If File.Exists(strDatei) Then
    4. File.delete(strDatei)
    5. Else
    6. 'Irgendwas
    7. End If


    // EDIT: Oops, hochkomma vergessen.. korrigiert :rolleyes:

    // EDIT2: "Then" hinzugeügt.
    "Wenn jemand in einem Betrieb unverzichtbar ist, dann ist dieser Betrieb falsch organisiert." - Roberto Niederer

    Dieser Beitrag wurde bereits 4 mal editiert, zuletzt von „milaim“ ()

    hallo,

    Naja mag sein das es ohne True/False kürzer ist aber es ist meiner Meinung nach kein schlechter Stil. Hat finde ich nichts damit zu tun.

    da muss ich kevin89, mal ausnahmsweise zustimmen.

    gruss

    mikeb69

    Hmm ihr sprecht über Stil ...
    Da ich eigentlich aus dem Bereich der Borlandsprachen komme (Delphi, C++) halte ich mich auch in VB an den Styleguide von Delphi.
    Ist halt eine Angewohnheit ^^

    Hier mal was zum Thema CodingStyle:
    StyleGuide

    Es gibt auch einen von Microsoft (mich gruselt es schon wenn ich daran denke -.-) den kann aber, soweit ich das gesehen habe, nur als Buch kaufen.
    Leider wird dort z.B. die Ungarische Notation nicht so eingesetzt wie es vom Erfinder gedacht war.
    Was natürlich einheitliche Standards schwer macht.

    Ich selbst verwende die Ungarische Notation nur bei Steuerelementen und dann auchnur die Variante "Systems Hungarian".

    Wer gern auch noch lesen möchte:
    Borland Styleguide in deutsch