StarDateCalc - Klassenbibliothek NuGet

Kirk1701A · Verfasst: Do 04.10.18 11:57

Hallo Leute,

ich habe das genaueste NuGet-Paket für Sternzeitberechnungen veröffentlicht und zum Download (im NuGet-Manager), das jemals veröffentlicht wurde. Es werden die Sternzeiten berechnet, die in StarTrek Verwendung haben. Es gibt 2 Sternzeittypen; TOS- und TNG-Sternzeit.

In beiden kann man entweder nichts eingeben in den Parameter (was dann zum aktuellen Datum und Uhrzeit führt) oder man muss vom Jahr über den Tag bis hin zur Minute alles eingeben, um die Sternzeit für das gewünschte Datum zu berechnen.

Viel Spass

LLAP
Euer Kirk

www.nuget.org/packages/StarDateCalc/

_________________
"Ich war es und werde es immer bleiben... Ihr Freund!"
@Spock -> Star Trek II: Der Zorn des Khan

Palladin007 · Verfasst: Do 04.10.18 14:34

Ich persönlich hab keine Ahnung von dem Thema, also kann ich nicht beurteilen, wie gut das tatsächlich funktioniert, aber ich kann den Code beurteilen.
Also hab ich mir das Package herunter geladen und decompiled.

Ein paar Kritik-Punkte:

Der Code beider Klassen sieht größtenteils nach Copy&Paste aus. Das ist für die Nutzer des Packages kein Problem, aber für dich für zukünftige Anpassungen schon, denn Du machst immer min. doppelte Arbeit.
Allgemein ist mir aber auch der Sinn beider Klassen nicht klar, aber das kann auch an meinem fehlenden Detail-Wissen liegen, die Berechnung habe ich mir nicht im Detail angeschaut.
Beim grob überfliegen fällt mir nur auf, dass der Subtrahend vor der Multiplikation mit 1000 je anders ist (2323 bzw. 2063), das kann man auch als Parameter für eine private Methoden-Überladung mitgeben.
Beide Klassen enthalten ausschließlich statische Member, inklusive statischer Properties. Mir erschließt sich der Sinn nicht, aber ich kann mit absoluter Sicherheit sagen, dass es bessere Wege gibt und dass das in einem MultiThreading-Scenario Probleme machen wird. Es gibt auch keine Doku, die darüber informieren würde, nichts deutet darauf hin, dass die Klasse nicht in einem MultiThreading-Scenario verwendet werden darf.
Du könntest einfach Instanz-Member daraus machen und die Parameter im Konstruktor angeben, dann kann man wenigstens die Klasse über mehrere Threads hinweg nutzen.
Noch besser wäre, wenn es gar keine Properties gibt, sondern einfach die Parameter verwendet oder vorher in lokale Variablen geschrieben werden. Dann ist jeder Methoden-Aufruf für sich eigenständig und von der Umgebung unabhängig, ideal für ein MultiThreading-Scenario.
Die CheckLeapYear-Methode ist schlicht falsch.
.NET hat dafür DateTime.IsLeapYear
Du kannst dir auch den Source-Code dazu anschauen, dann siehst Du, was Du falsch gemacht hast.
Allein wegen diesem Fehler kann die Behauptung, dass es die genaueste Berechnung sein soll, nicht stimmen.
Warum sind die Parameter ausgeschrieben, warum nicht einfach DateTime? Das bringt schon einige nützliche Dinge mit und jeder nutzt DateTime. Für die Überladung, die das aktuelle Datum übergibt, kannst Du dann auch einfach DateTime.Now verwenden.
So wie Du den useCurrentDate-Parameter verwendest, tut die Methode zwei Dinge (siehe Single Responsibility Principle), beide Male das Gleiche. Da die public-Methoden beide je true und false übergeben, können die einzelnen Code-Abschnitte doch auch direkt in dieser Methode landen. Du sparst eine Methode und brauchst die Properties nicht mehr.
Aber wenn ich das beim grob überfliegen richtig sehe, brauchst Du das gar nicht, es reicht eine Methode, die ein DateTime bekommt und für die Variante für das aktuelle Datum wird einfach DateTime.Now verwendet.
double ist nicht geeignet für Berechnungen, die eine hohe Genauigkeit fordern, dafür ist decimal konzipiert. Wenn Du ".0" anhängst, ist das automatisch double, wenn Du ein "M" an jede Zahl anhängst, wird die Zahl als decimal gewertet.

Allgemein gibt es am Code noch Dinge, die ich anders, aus meiner Sicht "sauberer" machen würde, allerdings kann ich nicht mit Sicherheit sagen, ob das nicht dem Compiler zu verdanken ist.

Ich hab mir den TNG-Code vom Decompiler geschnappt und meine Punkte darauf angewandt:

markieren

C#-Quelltext

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:

public static decimal CalcStardateTNG()
{
    return CalcStardateTNG(DateTime.Now);
}
public static decimal CalcStardateTNG(DateTime dateTime)
{
    decimal num8, num9;

    if (DateTime.IsLeapYear(dateTime.Year))
    {
        num8 = dateTime.Day + (153M * dateTime.Month - 157M) / 5M;
        num9 = 366M;
    }
    else
    {
        num8 = dateTime.Day + (153M * dateTime.Month - 162M) / 5M;
        num9 = 365M;
    }

    return 1000M * (dateTime.Year + 1M / num9 * (num8 - 1M + dateTime.Hour / 24M + dateTime.Minute / 1440M) - 2323M);
}

Verzeiht bitte die Benennung und Formatierung der Berechnung, das verdanke ich dem Compiler, ich hab immer noch keine Ahnung, was da gerechnet werden soll

Das Ergebnis ist bis auf Nachkommastelle 8 bzw. 9 (je nach Rundung) dasselbe wie bei deiner Berechnung (mit korrigierter CheckLeapYear-Methode), die folgenden 14/15 Nachkommastellen sind natürlich anders, da double keine weiteren 14/15 Nachkommastellen hat.

Die Korrektur der CheckLeapYear-Methode:

markieren

C#-Quelltext

1:
2:
3:
4:

private static bool CheckLeapYear(double year)
{
    return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
}

Kirk1701A · Verfasst: Do 04.10.18 20:41

Hallo,

dankeschön, dass du mir gesagt hast, was im Code nicht ganz ok ist.

Das NuGet habe ich nochmal getestet und es funktioniert einwandfrei. Ich gebe zu, der Code ist nicht gerade der schönste, aber ich wollte das schnell draussen haben.

Geplant ist, dass ich eine "Chefklasse" erstelle, von der aus alles gemanaget wird. Nur, wie kann ich die TNG bzw. TOS Klassen so einstellen, dass dort NUR die "Chefklasse" drauf zugreifen darf. Das weiss ich nicht umzusetzen.

Auch meine Kommentierung ist etwas mau.

Wie schon gesagt, eine große Baustelle, was den Code betrifft, aber das Ergebnis ist voll ok.

LLAP
Euer Kirk

_________________
"Ich war es und werde es immer bleiben... Ihr Freund!"
@Spock -> Star Trek II: Der Zorn des Khan

Palladin007 · Verfasst: Do 04.10.18 21:31

Zitat:

Das NuGet habe ich nochmal getestet und es funktioniert einwandfrei. Ich gebe zu, der Code ist nicht gerade der schönste, aber ich wollte das schnell draussen haben.

"schnell" ist selten eine gute Idee...

Zitat:

Geplant ist, dass ich eine "Chefklasse" erstelle, von der aus alles gemanaget wird. Nur, wie kann ich die TNG bzw. TOS Klassen so einstellen, dass dort NUR die "Chefklasse" drauf zugreifen darf. Das weiss ich nicht umzusetzen.

Was meinst Du mit "Chefklasse"? Das kann eine gute Idee sein, aber ebenso auch eine sehr schlechte Idee, besonders im Hinblick auf Abhängigkeiten.

Ich persönlich würde das ähnlich der Math-Klasse aufbauen. Pro Berechnung eine statische Methode, keine statischen Properties oder Felder, keine Instanz-Member. Jede Methode muss für sich alleine eigenständig arbeiten und darf keine Abhängigkeiten haben, die nicht in Form von Parametern (z.B. DateTime und das ist mMn. keine nennenswerte Abhängigkeit) mit gegeben werden.

markieren

C#-Quelltext

1:
2:
3:
4:
5:

public static class StardateMath
{
    public static decimal CalcTNG(DateTime dateTime) { /* ... */ }
    public static decimal CalcTOS(DateTime dateTime) { /* ... */ }
}

Zitat:

Auch meine Kommentierung ist etwas mau.

Ich persönlich betrachte Kommentare nicht als allgemeine Pflicht, mMn. gehören sie dort hin, wo etwas nicht auf Anhieb verständlich ist. Bei komplexen Berechnungen schreibe ich z.B. in Form von Kommentaren die Formeln bzw. die Herleitung und entsprechende Erklärungen vor die Berechnung, da sich sowas bedeutend leichter lesen lässt, als die Berechnung in C#.

Außerdem kann ich deine Kommentare nicht sehen, Du hast den Code nicht veröffentlicht. Ich sehe nur das, was der Compiler daraus macht.

Kirk1701A · Verfasst: Fr 05.10.18 01:26

Hallo,

mit "Chefklasse" meine ich eine übergeordnete Klasse, die auf die beiden momentan schon vorhandenen Klassen zugreift. Und NUR DIESE Klasse soll auf die untergeordneten zugreifen können.

LLAP
Euer Kirk

_________________
"Ich war es und werde es immer bleiben... Ihr Freund!"
@Spock -> Star Trek II: Der Zorn des Khan

Palladin007 · Verfasst: Fr 05.10.18 08:56

Das geht nur, wenn die untergeordneten Klassen private in dieser "Chefklasse" sind.
Oder Du machst die untergeordneten Klassen internal, dann kannst Du nur in der Assembly darauf zugreifen.
Per Reflection kann man natürlich immer darauf zugreifen.

Aber wie gesagt:
Ich halte das für völlig unnötig und sinnfrei, es reicht aus, daraus zwei Methoden zu machen - oder mit entsprechender Abstraktion eine Methode mit Überladungen.
Und wenn Du da noch mehr Features einbauen willst und deshalb Klassen brauchst, dann mach wenigstens Instanz-Member daraus.

Kirk1701A · Verfasst: Mi 10.10.18 20:40

Hallo Leute,

ich habe vor wenigen Tagen ein Update veröffentlicht. Ich habe im Prinzip alles umgekrempelt. Mal sehen, was noch verbessert werden kann.

Ich bin schon an der Entwicklung von Version 2.0.0, aber mehr möchte ich noch nicht sagen...

Was aber ein Forum, dessen Namen ich nicht nennen darf aber bestätigt, ist, dass die berechneten Sternzeiten absolut exakt und richtig sind.

LLAP
Euer Krik

_________________
"Ich war es und werde es immer bleiben... Ihr Freund!"
@Spock -> Star Trek II: Der Zorn des Khan

Stephan74656 · Verfasst: Mi 10.10.18 20:55

Hi Kirk,

ich bin echt begeistert von deinem NuGet-Paket. Wie du sagst, ist das der genaueste Sternzeitrechner.

Ich bin selbst ein StarTrek-Fan und echt verrückt danach. Ich konnte dank deinem NuGet einen absolut todsicheren und exakten Sternzeitrechner als Konsole und WPF erstellen.

Stephan74656

Palladin007 · Verfasst: Do 11.10.18 01:17

Ich würde immer noch mit DateTime arbeiten als mit fünf einzelnen Parametern.
Was für einen Vorteil haben denn Bytes im Vergleich zu den Ints, die DateTime verwendet?

Auch das CheckLeapYear ist immer noch fehlerhaft, oder funktionieren die Schaltjahre bei Startrek anders?

Und ich würde in dem Fall wieder eine statische Klasse daraus machen, jetzt sind die Methoden ja alle in sich abgeschlossen, sie haben keinen Status und sie sind von nichts abhängig.
Genau so sollte es für solche Methoden sein und in so einem Fall spricht nichts gegen static. Ich würde es sogar bevorzugen, aber das ist wahrscheinlich Geschmackssache.

Über den Code selber hab ich ja schon alles gesagt.

Stephan74656 · Verfasst: Do 11.10.18 21:18

Hallo,

also ich persönlich finde das mit den Byte-Parametern sehr praktisch. Man kann dadurch (bei großen Codes) ein bisschen der Ressourcen sparen. Auch kann man die Informationen direkt eingeben und nicht in DateTime konvertieren.

Und wie gesagt...

Es ist einfach spitze :flehan:

	Mitgliederliste
	Gruppen
	Das Team
	Richtlinien
	Synonyme