Bitte um Kritik zu einer meiner C++ Library

_ro_ro

Hallo,

nach 4 Wochen C++ habe ich so Manches fertiggestellt und bitte mal um Kritik:
http://rolfrost.de/datecalc.chtml
(also nicht die Anwendung an sich sondern die Library, Link unten ebenda).

MFG

DocShoe

Die kurze und höfliche Anwort: verbesserungswürdig.
Die nüchterne Antwort: ´ne Katastrophe. Dabei bewerte ich nur den Code unabhängig von deinem Kenntnisstand, es ist klar, dass man nach 4 Wochen C++ noch keinen guten C++ Code schreiben kann.

was heißt "Scaliger", was bedeutet der Begriff?
warum ist alles einer Headerdatei implementiert?
keine Datenkapselung, alle Member und Funktionen sind public
warum ist das Ganze überhaupt eine Klasse? Das kann man auch als freie Funktion implementieren, die eine Struktur mit Datumsinformationen zurückgibt.
du wirfst immer noch std::string statt des passenden Exceptiontyps.
fehlende const-correctness. Überall.
du verwendest überall Magic Numbers statt aussagekräftiger Konstanten
die Variablennamen verraten nichts über ihre Bedeutung. Warum gibt es deutsche und englische Bezeichnungen?
du deklarierst eine Variable und weist ihr dann einen Wert zu. Warum keine direkte Initialisierung?
du benutzt C-Arrays statt std::array, warum?
du benutzt C-style casts statt der C++ casts, warum?
warum benutzt du intwo boolder korrekte Datentyp wäre? Oder noch besser enum class.
fehlende Qualifikation von std::string und keine include-Anweisungen, das dürfte so nichtmal übersetzen
das Array der Wochentage sollte static const sein, damit`s nicht bei jedem Aufruf neu erzeugt werden muss. Eigentlich bräuchtest du auch eine Art locale, falls du doch mal Wochentage in anderen Sprachen ausgeben möchtest.
die Funktion int julianday_to_date(int dmy[], int jd) ist schlimm. Warum hat sie einen Rückgabewert und einen Referenzparameter, der auch noch verändert wird? Warum wird das gesamte Ergebnis nicht mit dem passenden Typen zurückgeben?
dass das Jahr ein signed Datentyp ist ok, aber warum sind alle anderen Datenfelder auch signed?
warum hört dein gregorianischer Kalender im Jahr 10.000 auf?

wob

Schreibe vom Handy, kann hab die lib noch nicht gesehen, aber kann dir die Frage nach Scaliger beantworten:

https://de.m.wikipedia.org/wiki/Julianisches_Datum

Ein erster Schritt zum heutigen Julianischen Datum erfolgte mit dem 1583 erschienenen Buch De emendatione temporum des französischen Humanisten Joseph Scaliger

Nehme an, der ist gemeint.

_ro_ro

@DocShoe sagte in Bitte um Kritik zu einer meiner C++ Library:

Die kurze und höfliche Anwort: verbesserungswürdig.
Die nüchterne Antwort: ´ne Katastrophe. Dabei bewerte ich nur den Code unabhängig von deinem Kenntnisstand, es ist klar, dass man nach 4 Wochen C++ noch keinen guten C++ Code schreiben kann.

was heißt "Scaliger", was bedeutet der Begriff?

Habe ich ergänzt,

keine Datenkapselung, alle Member und Funktionen sind public

Das ist so beabsichtigt. Private ist hier weder sinnvoll noch nötig.

warum ist das Ganze überhaupt eine Klasse?

Es gibt 3 Konstruktoren, je nach Liste der Argumente. Beim Erstellen der Instanz erfolgt eine Validierung. Geplant ist. u.a. ein Overload der Operatoren + und ++ sowie -. So kann man ganz einfach Tage inkrementieren oder Differenzen zwischen Datierungen über Instanzen berechnen.

du wirfst immer noch std::string statt des passenden Exceptiontyps.

Ja sicher doch. Das erscheint mir mit Blick auf die Anwendungen auch am zweckmäßigsten. Denn diese Texte soll ja ein gewöhnlicher Benutzer verstehen, der will nur wissen ob er ein gültiges Datum eingegeben hat.

fehlende const-correctness. Überall.

Ach ja.

du verwendest überall Magic Numbers statt aussagekräftiger Konstanten

kommt noch. 0 Julianisch, 1 Gregorianisch.

die Variablennamen verraten nichts über ihre Bedeutung. Warum gibt es deutsche und englische Bezeichnungen?

Nach außen hin ist alles deutsch.

du deklarierst eine Variable und weist ihr dann einen Wert zu. Warum keine direkte Initialisierung?

Initialisierung ist Sache des Konstruktors. Da habe ich hier gelernt.

du benutzt C-Arrays statt std::array, warum?

Weil diese Lib keine Array-Methoden und von daher auch keine Array-Objekte braucht.

du benutzt C-style casts statt der C++ casts, warum?

Ist eine Altlast. Könnte man verbessern.

warum benutzt du intwo boolder korrekte Datentyp wäre? Oder noch besser enum class.

Für eine public Datum-Validate-Methode ist bool sicher sinnvoll.

fehlende Qualifikation von std::string und keine include-Anweisungen, das dürfte so nichtmal übersetzen

Diese Lib wird im Rahmen eines Web-Application-Framework eingebunden und auch darüber kompliliert. Ich habe nicht vor, diese Lib als .so Datei auszulagern.

das Array der Wochentage sollte static const sein, damit`s nicht bei jedem Aufruf neu erzeugt werden muss.

Es gibt nur einen Aufruf: Wenn die Instanz erstellt wird. Danach findet sich der Wochentag in einer Eigenschaft und von daher sind weitere Aufrufe überflüssig.

die Funktion int julianday_to_date(int dmy[], int jd) ist schlimm. Warum hat sie einen Rückgabewert und einen Referenzparameter, der auch noch verändert wird?

Der Rückgabewert gibt an, ob Gregorianisch oder Julianisch gerechnet wurde.

warum hört dein gregorianischer Kalender im Jahr 10.000 auf?

Das ist eine willkürliche Festlegung meinerseits um eine Garantie zu geben daß die Berechnung stimmt.. Nach J.J. Scaliger würde er sogar noch früher aufhören. Ansonsten beruhen sämtliche Kalenderberechnungen stets auf Annahmen die man trifft.

Im Übrigen rechen Astronomen weltweit mit denselben Formeln. Wo u.a. die Annnahme drinsteckt daß das Jahr 4713 B.C. ein Schaltjahr war

Viele Grüße!!!

omggg

@wob sagte in Bitte um Kritik zu einer meiner C++ Library:

Schreibe vom Handy, kann hab die lib noch nicht gesehen, aber kann dir die Frage nach Scaliger beantworten:

https://de.m.wikipedia.org/wiki/Julianisches_Datum

Ein erster Schritt zum heutigen Julianischen Datum erfolgte mit dem 1583 erschienenen Buch De emendatione temporum des französischen Humanisten Joseph Scaliger

Nehme an, der ist gemeint.

Ehm, vielleicht bin ich gerade etwas begriffsstutzig, aber welche Einheit soll der erfunden haben? Da hätten auch schöne Kuhnamen stehen können, käme aufs Gleiche heraus.

DocShoe

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

Das ist so beabsichtigt. Private ist hier weder sinnvoll noch nötig.

Achso. Dann kommt dieser Fall definitiv auch nie vor, richtig?

Scaliger s( 17, 1, 2024 );
s.day = -7;

Dann muss ich meine Antworten revidieren, deine Klasse ist für ihren Anwendungsfall perfekt, da muss nichts weiter gemacht werden. Da sie nur intern benutzt wird greifen hier die üblichen OOP Paradigmen nicht.
Viel Erfolg bei der weiteren Frickelei.

_ro_ro

@DocShoe sagte in Bitte um Kritik zu einer meiner C++ Library:

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

Das ist so beabsichtigt. Private ist hier weder sinnvoll noch nötig.

Achso. Dann kommt dieser Fall definitiv auch nie vor, richtig?
Scaliger s( 17, 1, 2024 );
s.day = -7;

Richtig.

Schlangenmensch

@_ro_ro Sorry, aber das ist doch Blödsinn. Du nennst es library und stellst es Public zum Download bereit. Damit muss man doch davon ausgehen, dass du gewillt bist, die library auch für andere Entwckler zur Verfügung zu stellen. Üblicherweise gestaltet man dann das Public Interface so, dass man nicht groben Unfug damit anstellen kann.

Wenn du kein Interesse an Feedback, auch betreffend best practise und Style hast, verstehe ich den Sinn dieses Threads nicht

_ro_ro

@Schlangenmensch sagte in Bitte um Kritik zu einer meiner C++ Library:

@_ro_ro Sorry, aber das ist doch Blödsinn. Du nennst es library und stellst es Public zum Download bereit.

Nein. Ich stellte es zur Kritik bereit.

Damit muss man doch davon ausgehen, dass du gewillt bist, die library auch für andere Entwckler zur Verfügung zu stellen.

Für Entwickler. Ja.

Wenn du kein Interesse an Feedback, auch betreffend best practise und Style hast, verstehe ich den Sinn dieses Threads nicht

Den Unterschied zwischen Kritik und Beleidigung kennen Sie?

MFG

hat ja nicht lang gedauert bist du dich wieder selbst entlarvt hast...

Schlangenmensch

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

Den Unterschied zwischen Kritik und Beleidigung kennen Sie?

In online Foren ist das du Standard, kannst also gerne dabei bleiben.

Wenn du hier Code zum Review rein stellst gucken da Leute drauf, die mit C++ Entwicklung ihr Geld verdienen und die wahrscheinlich auch Reviews von Kollegen machen. Deren Prämisse ist: was wäre, wenn ich, oder andere Entwckler, mit dem Code weiter arbeiten müsste. Was hätte ich da gerne anders, damit das möglichst reibungslos funktioniert.

Ein entsprechendes Feedback hat @DocShoe dir gegeben.

Die Kritikpunkte hast du damit abgeschmettert, mit dem Argument dass das alles ja nur für dich intern ist und hast nicht mal versucht zu verstehen, woher die Kritik kommt.

Ich persönlich würde mich da als Kritikgeber leicht verarscht vorkommen, denn die Zeit die das aus Freundlichkeit rein investiert wurde, war ja offensichtlich verschenkt.

DocShoe

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

@Schlangenmensch sagte in Bitte um Kritik zu einer meiner C++ Library:

@_ro_ro Sorry, aber das ist doch Blödsinn. Du nennst es library und stellst es Public zum Download bereit.

Nein. Ich stellte es zur Kritik bereit.

Nein, tust du nicht. Du willst keine Kritik, sondern hören, was für'n toller Hecht du bist dass du nach 4 Wochen C++ schon so guten Code schreiben kannst. Dunning-Kruger lässt grüßen.

_ro_ro

@Schlangenmensch

Dann erkläre mir doch einmal bitte was ein professioneller Entwickler erwartet wenn er willkürlich die Eigenschaft einer Klasseninstanz ändert.

MFG

_ro_ro

Ein paar Schlüsse aus dem bisher kommuniziertem:

die numerische Eigenschaft day werde ich nicht mehr nach außen sichtbar machen. Stattdessen wird es einen getter geben welcher den Tag als String liefert.
mit allen anderen Eigenschaften werde ich gleichermaßen verfahren. Denn alles was nach draußen (UI) geht, sind Strings.
Außerhalb der Methoden gibt es nichts zu berechnen sondern nur IO und IO heißt String.
Gerechnet wird nur in Methoden.

Und weiterhin danke für Meinungen, Statements und Hinweise. MFG

_ro_ro

@Schlangenmensch

Haben Sie denn versucht, meine Antworten zu verstehen? Warum ich z.B. eine string-Exception werfe?

Und ja, ich denke schon daß day, mon, und year selbstsprechende Variablen sind.

MFG

firefly

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

@Schlangenmensch

Haben Sie denn versucht, meine Antworten zu verstehen? Warum ich z.B. eine string-Exception werfe?

MFG

Und du solltest dich mal mit std::exception selbst und von dieser abgeleiteten klassen von std::exception beschäftigen.
Alle direkt von std::exception abgeleitete klassen haben alle einen konstructor der einen string als parameter nimmt

Jockelx

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

Dann erkläre mir doch einmal bitte was ein professioneller Entwickler erwartet wenn er willkürlich die Eigenschaft einer Klasseninstanz ändert.

Geht es darum:

s.day = -7;

?

Meiner Erfahrung nach passieren die wenigsten Fehler beim Erstellen neuer Funktionen, sondern beim Erweitern von (Fremd-) Code.
Hier könnte z.B. sowas passieren:

//Version 1.0.0: Setze Datum
int day = 4;
s.day = day;

// Version 1.0.1: Gaaanz viel weiterer Krams
int day = 4;
// ...Krams
s.day = day;

// Version 1.0.2: Day muss vom Benutzer eingegeben werden
int day;
cin << day;
// ...Krams der verdeckt, dass die Lib offenbar keine Lust hat Fehler abzufangen
s.day = day;

Die Entwickler waren vielleicht nicht sehr gewissenhaft, aber boshaft war keiner. Trotzdem ist der Code Schrott und der Projektleiter entschuldigt sich, dass er eine Lib vorgeschlagen hat, die noch nicht mal versucht fehlerhafte Handhabung abzufangen.

omggg

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

@Schlangenmensch

Haben Sie denn versucht, meine Antworten zu verstehen? Warum ich z.B. eine string-Exception werfe?

Und ja, ich denke schon daß day, mon, und year selbstsprechende Variablen sind.

MFG

@_ro_ro Wenn man nach einem Review fragt und Hilfe haben möchte, dann muss man auch mit kritischen Antworten zurechtkommen... Du bist doch nicht aus Zucker, und es ist auch noch kein Meister vom Himmel gefallen.

mon für Monat ist denkbar schlecht gewählt und dein err-Handling auch.

_ro_ro

@firefly sagte in Bitte um Kritik zu einer meiner C++ Library:

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

@Schlangenmensch

Haben Sie denn versucht, meine Antworten zu verstehen? Warum ich z.B. eine string-Exception werfe?

MFG

Und du solltest dich mal mit std::exception selbst und von dieser abgeleiteten klassen von std::exception beschäftigen.
Alle direkt von std::exception abgeleitete klassen haben alle einen konstructor der einen string als parameter nimmt

Und was ist dann besser, als ein einfaches throw string("Datum ungültig") ?

_ro_ro

@omggg sagte in Bitte um Kritik zu einer meiner C++ Library:

@_ro_ro sagte in Bitte um Kritik zu einer meiner C++ Library:

@Schlangenmensch

Haben Sie denn versucht, meine Antworten zu verstehen? Warum ich z.B. eine string-Exception werfe?

Und ja, ich denke schon daß day, mon, und year selbstsprechende Variablen sind.

MFG

@_ro_ro Wenn man nach einem Review fragt und Hilfe haben möchte, dann muss man auch mit kritischen Antworten zurechtkommen... Du bist doch nicht aus Zucker, und es ist auch noch kein Meister vom Himmel gefallen.

mon für Monat ist denkbar schlecht gewählt und dein err-Handling auch.

Warum ist das err-Handling schlecht?