Da man häufiger User sieht die (funktionierenden) Code posten welcher aber leider eher schlecht als recht umgesetzt wurde, dachte ich mir mal ein Code-Korrektur-Sammel-Thread zu eröffnen. Grade als Programmieranfänger hat man es ja schwer den eigenen Code einzuschätzen - funktionieren ist eben nicht alles.
Das soll hier möglichst kein Diskussion-Thread werden. Wenn ein Helfer Fehler macht kann man ja eine PN statt einen Post schreiben - nur so bleibt hier eine gewisse Übersicht bestehen. Wenn der Thread gut läuft hätte man irgendwann eine Art Sammlung "guter Code vs. schlechter Code" von der man wiederum viel lernen kann.
Wie stelle ich mir das hier vor?
Der Hilfe suchende: postet einen Code-Ausschnitt der folgende Kriterien erfüllt:
- der Code muss
- der Code "funktioniert" aber man ist sich nicht sicher ob er auch gut umgesetzt ist
- keine gezielte Frage zum Code, dafür ist das Forum besser geeignet!
- ein relativ kurzer Code-Ausschnitt der ein bestimmtes Problem löst - niemand will hier Romane verbessern. Wer alles in eine 3km hohe Methode packt, dem ist nicht mehr zu helfen^^
- Controls und Variablen sollten anständig benannt sein. Evtl das ein oder andere Kommentar setzten, damit klar wird um welche Controls es sich handelt
- Der Code muss anständig formatiert sein! Es gibt einen Vorschau-Button um das vor dem posten zu prüfen und eine kleine Anleitung für Unwissende
- Eine kleine Beschreibung was der Code überhaupt bezwecken soll wäre angebracht
Der Helfende: Er und seine Antwort sollte auch ein paar Punkte erfüllen:
- Helfer soll mehr Wissen haben als der Hilfsbedürftige - man geht schließlich auch nicht ins Cockpit um dem Piloten zu sagen wie er fliegen soll
- möglichst keine "Einzeiler-Antworten" (außer der Code ist wirklich hoffnungslos oder fehlerfrei)
- den bestehenden Code übernehmen und entsprechend kommentieren - was wurde gut/schlecht umgesetzt, wo sind die Fehler? Was könnte man anders machen, etc
- vllt auch eine abgeänderte oder eigene Version posten - aber bitte mit Kommentaren UND Option Strict On
- evtl nicht alles zu hart anpacken. Die groben Fehler raus, Kleinigkeiten ( My-Namespace statt System.IO etc ) aufzeigen.
- wenn möglich ein paar hilfreiche Links nennen
Beispiel
Der 'Hilfsbedürftige' (da muss ein VBPler den Kopf für hinhalten, sry^^):
Spoiler anzeigen
Der Helfer:
Spoiler anzeigen
Ich hoffe der Thread wird seinen Zweck erfüllen. lg FreakJNS
Das soll hier möglichst kein Diskussion-Thread werden. Wenn ein Helfer Fehler macht kann man ja eine PN statt einen Post schreiben - nur so bleibt hier eine gewisse Übersicht bestehen. Wenn der Thread gut läuft hätte man irgendwann eine Art Sammlung "guter Code vs. schlechter Code" von der man wiederum viel lernen kann.
Wie stelle ich mir das hier vor?
Der Hilfe suchende: postet einen Code-Ausschnitt der folgende Kriterien erfüllt:
- der Code muss
Option Strict On
programmiert sein - wird langweilig immer wieder zu sagen dass 'String + Double kein Integer ergibt'- der Code "funktioniert" aber man ist sich nicht sicher ob er auch gut umgesetzt ist
- keine gezielte Frage zum Code, dafür ist das Forum besser geeignet!
- ein relativ kurzer Code-Ausschnitt der ein bestimmtes Problem löst - niemand will hier Romane verbessern. Wer alles in eine 3km hohe Methode packt, dem ist nicht mehr zu helfen^^
- Controls und Variablen sollten anständig benannt sein. Evtl das ein oder andere Kommentar setzten, damit klar wird um welche Controls es sich handelt
- Der Code muss anständig formatiert sein! Es gibt einen Vorschau-Button um das vor dem posten zu prüfen und eine kleine Anleitung für Unwissende
- Eine kleine Beschreibung was der Code überhaupt bezwecken soll wäre angebracht
Der Helfende: Er und seine Antwort sollte auch ein paar Punkte erfüllen:
- Helfer soll mehr Wissen haben als der Hilfsbedürftige - man geht schließlich auch nicht ins Cockpit um dem Piloten zu sagen wie er fliegen soll
- möglichst keine "Einzeiler-Antworten" (außer der Code ist wirklich hoffnungslos oder fehlerfrei)
- den bestehenden Code übernehmen und entsprechend kommentieren - was wurde gut/schlecht umgesetzt, wo sind die Fehler? Was könnte man anders machen, etc
- vllt auch eine abgeänderte oder eigene Version posten - aber bitte mit Kommentaren UND Option Strict On
- evtl nicht alles zu hart anpacken. Die groben Fehler raus, Kleinigkeiten ( My-Namespace statt System.IO etc ) aufzeigen.
- wenn möglich ein paar hilfreiche Links nennen
Beispiel
Der 'Hilfsbedürftige' (da muss ein VBPler den Kopf für hinhalten, sry^^):
Hallo. Der Code soll prüfen ob eine Trial-Version meines Programms abgelaufen ist. In der Data_z.cfg wird dazu beim ersten Programmstart das Ablaufdatum (also Jetzt + Gültigkeitsdauer) hinterlegt und bei jedem Programmstart mit der aktuellen Zeit vergleichen. Ist die Gültigkeitsdauer überschritten wird das Programm einfach beendet.
VB.NET-Quellcode
- Option Strict On
- Imports System.IO
- Public Class Form1
- Dim Datum_Erststart As Date = Date.Now.Date
- Dim Datei As String
- Dim Pfad As String = "C:\Users\zeus\Desktop\Neuer Ordner (8)\Data_z.cfg"
- Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
- Label1.Text = CStr("Aktuelles Datum: " & Datum_Erststart)
- 'Ermitteln ob Datei vorhanden ist,wenn ja auslesen und vergleichen mit heutigem Datum!
- If My.Computer.FileSystem.FileExists(Pfad) Then
- Datei = File.ReadAllText(Pfad)
- Label2.Text = CStr("Ablaufdatum: " & Datei)
- If Datum_Erststart > CDate(Datei) Then
- MsgBox("Ihre Testphase ist abgelaufen")
- End
- Else
- End If
- Else
- Dim Msg, Number, StartDate As String
- Dim Days As Double
- Dim SecondDate As Date = Datum_Erststart
- Dim IntervalType As DateInterval
- IntervalType = DateInterval.DayOfYear 'Monat-Tag-usw- als Interval einstellbar !
- StartDate = CStr(Datum_Erststart) 'Zeit bei Start
- SecondDate = CDate(StartDate)
- Number = "20" 'Zeit in Tagen die hinzugerechnet wird (( DayOfYear ))
- Days = Val(Number)
- Msg = CStr(DateAdd(IntervalType, Days, SecondDate)) 'Neues erstelltes Datum (+20Tage)
- 'Datei erstellen und datum reinschreiben mit +20 Tage
- Dim txt As String = Msg
- Dim fss As StreamWriter = File.CreateText(Pfad) 'Später ins Gemüse wos keiner findet !
- fss.Write(txt)
- fss.Close()
- 'Ermitteln ob Datei vorhanden ist,wenn ja auslesen und vergleichen mit heutigem Datum!
- Datei = File.ReadAllText(Pfad)
- If Datum_Erststart > CDate(Datei) Then
- End
- End If
- End If
- 'Restzeit bis Ablauf !
- Dim Differenz_Zeit As Long = DateDiff("d", Datum_Erststart, Datei)
- Label3.Text = CStr("Restzeit: " & Differenz_Zeit)
- End Sub
- 'Label1 und Label2 sind nicht nötig !
- End Class
Der Helfer:
Hallo PersonXYZ (am besten immer den Namen nenne damit der Bezug erhalten bleibt). Hier erstmal die Dinge dir mir an deinem Code aufgefallen sind:
ACHTUNG: KURIOSITÄT Wenn in der cfg-Datei 'murks' steht wird das Programm am CDate scheitern und eine Exception werfen. Lustigerweise arbeitest du im Form_Load-Event und dieses hat ein spezielles Verhalten: es ignoriert die Exception einfach. Das bedeutet: Wenn die Datei existiert und einen ungültigen Inhalt hat wird dein Programm an dieser Stelle immer aus der Methode rausspringen und die Trial-Version somit niemals ablaufen.
Ich würde so vorgehen:
Der Code ist auch nicht 100% korrekt. Beispielsweise können immer noch Lese/Schreib-Fehler auftreten (z.B. wenn das Programm keine entsprechenden Rechte hat) die du abfangen solltest. Eine Textdatei ist auch nicht soo geeignet - ein Eintrag in den Settings wäre besser. Sicher ist das allerdings alles nicht - selbst ein "verstecken der Datei im Gemüse" ist unsicher und sorgt für verärgerte Nutzer wenn du ihnen das Dateisystem verhunzt^^ Um der angesprochenen Kuriosität zu entgehen ist es besser du arbeitest im Shown-Event (da fällt eine Exception nicht unter den Tisch) und/oder baust Try-Catch'es gezielt ein. TryParse-Methoden sind dem natürlich vorzuziehen.
lg
Links:
[VB.NET] TryCatch ist ein heißes Eisen
VB.NET-Quellcode
- Option Strict On
- Imports System.IO
- Public Class Form1
- Dim Datum_Erststart As Date = Date.Now.Date 'könnte auch in anderen Methoden verwendung finden - ansonsten: globale deklaration unnötig
- Dim Datei As String 'Die Variable wird NUR in der Prüf-Methode gebraucht. Globales deklarieren ist unangebracht
- Dim Pfad As String = "C:\Users\zeus\Desktop\Neuer Ordner (8)\Data_z.cfg" 'ganz schlecht, besser in den Settings oder relativ zur .exe speichern
- Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
- Label1.Text = CStr("Aktuelles Datum: " & Datum_Erststart) 'es gibt auch eine .toString-Methode^^
- If My.Computer.FileSystem.FileExists(Pfad) Then 'Die IO.FileInfo kann das auch - My-Namespace ist nicht so schön
- Datei = File.ReadAllText(Pfad) 'Variable komisch benannt. Caste doch am besten direkt zu Date, den Dateiinhalt braucht man eh nie wieder
- Label2.Text = CStr("Ablaufdatum: " & Datei) 'es gibt auch eine .toString-Methode^^
- If Datum_Erststart > CDate(Datei) Then 'das CDate würde wegfallen.. ACHTUNG: KURIOSITÄT xD
- MsgBox("Ihre Testphase ist abgelaufen")
- End
- Else
- End If
- Else
- Dim Msg, Number, StartDate As String 'warum deklarieren aber keinen Wert zuweisen? Bei lokalen Variablen finde ich das sehr unschön
- Dim Days As Double 'Days soll sicher ein Integer sein?
- Dim SecondDate As Date = Datum_Erststart 'warum wird hier eine Variable einfach nur verdoppelt? Benutze doch weiterhin Datum_Erststart
- 'Der Teil ist wirklich hässlich. Mit einer Timespan ließe sich das in zwei Zeilen machen - übersichtlich und nachvollziehbar
- Dim IntervalType As DateInterval
- IntervalType = DateInterval.DayOfYear
- StartDate = CStr(Datum_Erststart)
- SecondDate = CDate(StartDate)
- Number = "20"
- Days = Val(Number) 'Cint oder .TryParse ist besser - glaube Val ist eine dieser "bösen Funktionen" aus VB6-Zeiten
- Msg = CStr(DateAdd(IntervalType, Days, SecondDate))
- Dim txt As String = Msg 'schon wieder unnötigerweise eine Variable verdoppelt. Msg könnte statt txt weiterverwendet werden, du änderst ja nichts
- Dim fss As StreamWriter = File.CreateText(Pfad) '[Später ins Gemüse wos keiner findet !] <= Das ich nicht lache: Programmabsturz wahrscheinlich, ILSpy oder andere Tools und man hat den Pfad definitiv!
- fss.Write(txt)
- fss.Close()
- Datei = File.ReadAllText(Pfad) 'relativ trivial da du vor zwei zeilen ein dir bekanntes Datum in die Datei geschrieben hast
- If Datum_Erststart > CDate(Datei) Then 'CDate könnte auch hier wegfallen
- End
- End If
- End If
- 'Restzeit bis Ablauf !
- Dim Differenz_Zeit As Long = DateDiff("d", Datum_Erststart, Datei) 'DateDiff ist untypisiert - gefällt mir nicht. Timespan sollte auch klappen.
- Label3.Text = CStr("Restzeit: " & Differenz_Zeit) 'es gibt auch eine .toString-Methode^^
- End Sub
- End Class
ACHTUNG: KURIOSITÄT Wenn in der cfg-Datei 'murks' steht wird das Programm am CDate scheitern und eine Exception werfen. Lustigerweise arbeitest du im Form_Load-Event und dieses hat ein spezielles Verhalten: es ignoriert die Exception einfach. Das bedeutet: Wenn die Datei existiert und einen ungültigen Inhalt hat wird dein Programm an dieser Stelle immer aus der Methode rausspringen und die Trial-Version somit niemals ablaufen.
Ich würde so vorgehen:
VB.NET-Quellcode
- Option Strict On
- Public Class Form1
- Public Const PFAD_ABLAUFDATUM As String = "ablaufdatum.txt" 'die bessere Alternative: ein Eintrag in den Settings. Ist aber alles unsicher.
- Public Const DAUER_TRIAL_TAGE As Integer = 30 'Die Trial soll 30 Tage gültg sein
- Private Sub Form1_Load(sender As System.Object, e As System.EventArgs) Handles MyBase.Load
- 'Try
- trialCheck() 'prüft ob die Trial-Version abgelaufen ist und beendet das Programm ggf
- 'Catch ex As Exception
- ' 'ein Fehler könnte auf manipulation hinweisen.
- 'End Try
- End Sub
- Public Sub trialCheck()
- Dim istAbgelaufen As Boolean = False
- Dim fi As New IO.FileInfo(PFAD_ABLAUFDATUM)
- Dim ablaufDatum As Date = Now + TimeSpan.FromDays(DAUER_TRIAL_TAGE)
- If fi.Exists Then
- 'Die Datei existiert
- Using reader As New IO.StreamReader(fi.OpenRead)
- Dim tmp As String = reader.ReadToEnd
- ablaufDatum = Date.Parse(tmp) 'TryParse wäre besser. Ist die Datei fehlerhaft könnte man auf manipulation tippen und die Trial als abgelaufen ansehen
- istAbgelaufen = (ablaufDatum <= Now)
- reader.Close()
- End Using
- Else
- 'Die Datei existiert nicht
- Using writer As New IO.StreamWriter(fi.OpenWrite)
- 'ablaufDatum = Now + TimeSpan.FromDays(DAUER_TRIAL_TAGE)
- writer.Write(ablaufDatum)
- writer.Flush()
- writer.Close()
- End Using
- End If
- If istAbgelaufen Then
- MsgBox("die Trial-Version ist abgelaufen")
- Me.Close()
- Else
- Dim ts As TimeSpan = ablaufDatum - Now
- MsgBox("die Trial ist noch " & CInt(ts.TotalDays) & " Tage gültig")
- End If
- End Sub
- End Class
Der Code ist auch nicht 100% korrekt. Beispielsweise können immer noch Lese/Schreib-Fehler auftreten (z.B. wenn das Programm keine entsprechenden Rechte hat) die du abfangen solltest. Eine Textdatei ist auch nicht soo geeignet - ein Eintrag in den Settings wäre besser. Sicher ist das allerdings alles nicht - selbst ein "verstecken der Datei im Gemüse" ist unsicher und sorgt für verärgerte Nutzer wenn du ihnen das Dateisystem verhunzt^^ Um der angesprochenen Kuriosität zu entgehen ist es besser du arbeitest im Shown-Event (da fällt eine Exception nicht unter den Tisch) und/oder baust Try-Catch'es gezielt ein. TryParse-Methoden sind dem natürlich vorzuziehen.
lg
Links:
[VB.NET] TryCatch ist ein heißes Eisen
Ich hoffe der Thread wird seinen Zweck erfüllen. lg FreakJNS