Verbesserungsvorschläge für Code

  • VB.NET
  • .NET (FX) 4.5–4.8

Es gibt 41 Antworten in diesem Thema. Der letzte Beitrag () ist von ErfinderDesRades.

    Verbesserungsvorschläge für Code

    Hallo.

    Kann mir jemand Tipps geben, wie ich den folgenden Code verbessern könnte?

    Bitte um viel Kritik.

    Wäre nett, wenn ihr mir ein paar Alternativen nennen könntet und wie man diese umsetzen würde
    (ich möchte keinen konkreten Code, nur Beispiele).

    Dieser Beitrag wurde bereits 1 mal editiert, zuletzt von „Daniel Baumert“ ()

    Hast du den Code selber geschrieben?
    • msdn.microsoft.com/en-us/library/ms229002(v=vs.110).aspx . Bitte komplett lesen
    • ReadTableFromOffset(Offset As ULong) - Wieso einen ULong anfordern, wenn dieser dann zu Int konvertiert wird?
    • Kein ReDim benutzen
    • Statt And und Or AndAlso und OrElse nutzen
    • Datentypen generell überarbeiten. Du musst irgendwie dauernd alles casten. Wieso brauchst du da überhaupt dauernd einen Long? Meiner Erfahrung nach braucht man den hauptsächlich bei Größenangaben in Bytes
    • j = j + 1 > j += 1. Aber j ist jetzt nicht so aussagekräftig, siehe Punkt 1
    • Case CUShort(2) > Case 2US, siehe Literale: msdn.microsoft.com/de-de/library/s9cz43ek.aspx
    Mfg
    Vincent

    @Daniel Baumert Select Case encoding Select Case CUShort(encoding) Nimm für encoding Integer und gut.
    Statt Select Case kannst Du Dir auch für jedes Encoding eine von einer gemeinsamen Basisklasse abgeleitete Klasse machen, für jedes Encoding eine.
    Alles Gemeinsame in die Basisklasse, die Spezifika in die abgeleiteten Klassen. Das lässz sich einfacher erweitern.

    VB.NET-Quellcode

    1. Private Function ConvertToInteger(startIndex As Integer, Size As Integer) As ULong
    Meinst Du, dass dieser Name korrekt ist?
    usw.
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!

    Mit dem And und Or durch OrElse und AndAlso ersetzen habe ich gerade ein Problem:

    VB.NET-Quellcode

    1. Private Function ConvertToUL(startIndex As Integer, Size As Integer) As ULong
    2. If Size > 8 OrElse Size = 0 Then Return Nothing
    3. Dim retVal As ULong = 0
    4. For i As Integer = 0 To Size - 1
    5. retVal = ((retVal << 8) OrElse db_bytes(startIndex + i))
    6. Next
    7. Return retVal
    8. End Function


    Wenn ich dort (so wie gerade) das hier stehen habe:

    VB.NET-Quellcode

    1. retVal = ((retVal << 8 ) OrElse db_bytes(startIndex + i))


    Kommt die Meldung:

    VB.NET-Quellcode

    1. Option Strict On disallows implicit conversions from 'ULong' to 'Boolean'.


    Mit OR gehts ohne Fehler.

    Wie mache ich das?

    ja, das Schlüsselwort Or bezeichnet im Grunde 2 verschiedene Operatoren:
    1. Logisches Oder, und da sollte man immer OrElse bevorzugen
    2. Bitweises Oder - das ist natürlich unersetzlich.
    Aber generell wundere ich mich, dass du tatsächlich Verbesserungs-Vorschläge bekommst.
    Obwohl du kein Wort verlauten lässt, was der Code überhaupt soll - und ich weiß auch nicht, ob man ihn ohne diese Info überhaupt genügend verstehen kann, um Verbesserungen vorzuschlagen.

    ErfinderDesRades schrieb:

    das Schlüsselwort Or bezeichnet im Grunde 2 verschiedene Operatoren
    Nein.
    Or ist ausschließlich bitweise.
    Damit das in VB (6 / .NET) funktioniert, sieh Dir folgendes an:

    VB.NET-Quellcode

    1. Dim a As Integer = CInt(True)
    2. MessageBox.Show(a.ToString)

    C#-Quellcode

    1. //int a = (int)true; // geht nicht: Eine Konvertierung vom Typ 'bool' in 'int' ist nicht möglich.
    2. int b = Convert.ToInt32(true);
    3. MessageBox.Show(b.ToString());
    --------------------
    @Daniel Baumert Nimm eine List(Of T)
    ----
    Ich wundere mich gerade, dass trotz Option Strict On bei einer ULong-Funktion Return Nothing funktioniert.
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!

    RodFromGermany schrieb:


    Ich wundere mich gerade, dass trotz Option Strict On bei einer ULong-Funktion Return Nothing funktioniert.



    Könnte man die Funktion besser schreiben? Wenn ja, wie?


    Ich hab auch hier eine Funktion, die auch Nothing zurückgibt. Das kommt mir Spanisch vor:

    VB.NET-Quellcode

    1. Private Function GVL(st As Integer) As Integer
    2. If st + 8 > db_bytes.Length Then Return -1
    4. For i As Integer = st To st + 7
    5. If i > db_bytes.Length - 1 Then Return Nothing
    6. If (db_bytes(i) And &H80) <> &H80 Then Return i
    7. Next
    8. Return st + 8
    9. End Function

    Alternativen?

    RodFromGermany schrieb:

    Or ist ausschließlich bitweise.
    Mag sein. Aber wenn man Or auf Booleans anwendet, dann verwendet man es als logischen Operator, also um logische Aussagen treffen zu können (Verzweigungen, bedingte Schleifen).
    Und da würd ich empfehlen, das als prinzipiellen Unterschied aufzufassen und zu unterscheiden, auch wenn intern beides über Bit-Operationen abgewickelt werden mag.
    Und wenn man dann logisches und bitweises Or auseinander-hält, dann ists auch kein Problem mehr, zu entscheiden, welches Or man durch OrElse ersetzen sollte, und welches nicht: Nämlich zu ersetzen ist das logische Or.

    Daniel Baumert schrieb:

    Könnte man die Funktion besser schreiben? Wenn ja, wie?
    Mach Dir ein Validitätsflag oder nimm Nullable(Of Integer), das ist so was wie ein Pointer auf ein Integer, ist der Pointer valid, enthält er ein Ergebnis, ist er invalid, wars nix, gugst Du Nullable(Of T):

    VB.NET-Quellcode

    1. Private Function ConvertToUL(startIndex As Integer, Size As Integer) As Nullable(Of Integer)
    2. If Size > 8 OrElse Size = 0 Then Return Nothing
    3. Dim retVal As Integer = 0
    4. For i As Integer = 0 To Size - 1
    5. If (db_bytes(i) And &H80) <> &H80 Then Return i
    6. Next
    7. Return retVal
    8. End Function
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!

    @RodFromGermany, wenn ich meine Funktion durch deine ersetze, erhalte ich direkt einen bösen Fehler:

    VB.NET-Quellcode

    1. Private Sub ReadMasterTable(Offset As Integer)
    3. If db_bytes(Offset) = &HD Then 'Leaf node
    4. 'Length for setting the array length for the entries
    5. Dim Length As UShort = CUShort(ConvertToUL(CInt(Offset + 3), 2) - 1)


    Codezeile:

    VB.NET-Quellcode

    1. Dim Length As UShort = CUShort(ConvertToUL(CInt(Offset + 3), 2) - 1)



    Additional information: Die arithmetische Operation hat einen Überlauf verursacht.

    Offset hat den Wert 100

    Daniel Baumert schrieb:

    wenn ich meine Funktion durch deine ersetze
    Bei mir kommt "UShort" nicht vor.
    Lege Dich auf einen Zahlenbereich fest.
    Setz da einen Haltepunkt drauf und sieh Dir jede einzelne Komponente der Operation an.
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!

    RodFromGermany schrieb:

    Daniel Baumert schrieb:

    wenn ich meine Funktion durch deine ersetze
    Bei mir kommt "UShort" nicht vor.
    Lege Dich auf einen Zahlenbereich fest.




    Das ist die Big-Endian Konvertierung. Die Funktion sieht ja so aus:

    VB.NET-Quellcode

    1. Private Function ConvertToUL(startIndex As Integer, Size As Integer) As ULong
    2. If Size > 8 OrElse Size = 0 Then Return Nothing
    3. Dim retVal As ULong = 0
    4. For i As Integer = 0 To Size - 1
    5. retVal = ((retVal << 8) Or DataBaseBytes(startIndex + i))
    6. Next
    7. Return retVal
    8. End Function



    Aufrufe der Funktion sind z.B.:

    VB.NET-Quellcode

    1. Dim Length As UShort = CUShort(ConvertToUL(Offset + 3, 2) - 1)




    VB.NET-Quellcode

    1. Dim ent_offset As ULong
    2. For i As Integer = 0 To Length
    3. ent_offset = ConvertToUL(Offset + 8 + (i * 2), 2)



    VB.NET-Quellcode

    1. kd.root_num = CLng(ConvertToUL(CInt(ent_offset + Rec_Header_Size + Field_Size(0) + Field_Size(1) + Field_Size(2)), CInt(Field_Size(3))))


    VB.NET-Quellcode

    1. ReadMasterTable(CInt((ConvertToUL(Offset + ent_offset, 4) - 1) * page_size))



    usw.


    Wie muss deine Funktion jetzt ausssehen, damit das funktioniert?

    Daniel Baumert schrieb:

    Das ist die Big-Endian Konvertierung.
    What :?:
    Lass das den Text-Konverter allein machen.
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!

    Also das aus dem Stehgreif zu erklären, geht ohne Code denke ich mal schlecht.

    Also das ist eine Klasse, um Sqlite 3 Tables aus einer lokalen Datei auszulesen.

    Habe jetzt versucht alle eure bisher gegebenen Tipps umzusetzen.

    Bitte dennoch um weitere Verbesserungsvorschläge, ich denke da ist noch viel mehr Verbesserung möglich.

    Table_Entries in eine List(Of T) zu bekommen habe ich nicht geschafft, das gab irgendwie immer fehler.
    Bei allen anderen Arrays habe ich es geschafft.

    Dieser Beitrag wurde bereits 2 mal editiert, zuletzt von „Daniel Baumert“ ()

    Mach mal bitte um Deinen Code einen Spoiler (ehemals Expander).
    Spoiler anzeigen
    so was

    Daniel Baumert schrieb:

    Table_Entries
    Das kann z.B. daran liegen, dass in Deinem Code Table_Entries nicht vorkommt. :/
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!