Frage bzgl. OOP und einem Klassen Diagramm

[ebody]

Hallo,

ich arbeite jetzt nach ein paar Monaten Pause wieder an einem privaten Projekt, wo ich mit Klassen arbeite und Objekt orientiert arbeiten möchte. Um einen Überblick zu erhalten (sind nur 750 Codezeilen), habe ich versucht mir mal ein Klassen Diagramm zu erstellen.

.

Dabei haben sich einige Fragen ergeben:

1. Ist es sinnvoller zu erst ein solches Diagramm zu erstellen und dann zu programmieren? Damit man schon anhand des Diagramms erkennt, ob etwas Sinn macht, wie die Abhängigkeiten sein sollten?

2. Gibt es eine Regel, welche, wie viele Methoden in eine Klasse sollten und wann man besser eine weitere Klasse erstellt? Siehe z.B. die Klasse ReadSheet. Hier gibt es auch Methoden, die die Google Sheet URL und deren Rückgabe prüfen. Gehört das wirklich ins Objekt ReadSheet oder in eine neue Klasse wie CheckSheetUrl?

3. Für das Diagramm habe ich die VS Code Erweiterung "Draw.io Integration" verwendet. Gibt es hier oder in einer anderen UML Software die Möglichkeit, Kommentare für die Methoden zu hinterlegen, die man beim Mouseover z.B. im Diagramm sehen kann.

Mein Wunsch und Ziel ist es auch nach Monaten Abstand von einem Projekt schnell zu erkennen, was es kann, welche "Legosteine" (Klassen, Funktionen) ich zur Verfügung habe, welche ich zusammengebaut habe und ob ich einen Legostein anpassen/erweitern muss oder neuen brauche.

Gruß ebody

[dedlfix]

Tach!

1. Ist es sinnvoller zu erst ein solches Diagramm zu erstellen und dann zu programmieren? Damit man schon anhand des Diagramms erkennt, ob etwas Sinn macht, wie die Abhängigkeiten sein sollten?

Wenn es dir hilft, dann mach das. Es wäre dann auch sinnvoll, das immer mit dem Code synchron zu halten. Wenn du es (später) immer aus den Klassen automatisch erstellen lässt, dann musst du es nicht dauerhaft aufheben, weil du das ja jederzeit herstellen kannst, wenn du meinst, eins zu brauchen.

2. Gibt es eine Regel, welche, wie viele Methoden in eine Klasse sollten und wann man besser eine weitere Klasse erstellt?

Ja, diverse Leute haben diverse Faustregeln formuliert. Aber wichtiger als solche Regeln ist deine persönliche Einschätzung, wann etwas sinnvoll ist und wann nicht. Jedes Projekt hat seine eigenen Anforderungen, und nur du musst damit arbeiten. (Oder dein Team, aber dann musst du es mit dem Team absprechen.)

Wenn du Regeln brauchst, Prinzipien objektorientierten Designs listet viele auf. Die beschreiben aber auch eher qualitative Dinge und keine quantitativen.

Siehe z.B. die Klasse ReadSheet. Hier gibt es auch Methoden, die die Google Sheet URL und deren Rückgabe prüfen. Gehört das wirklich ins Objekt ReadSheet oder in eine neue Klasse wie CheckSheetUrl?

Vielleicht. Vielleicht auch nicht. Für Test Driven Design ist es sinnvoll, nicht zu viel in eine Einheit zu packen, weil der Testaufwand steigt. Für die allgemeine Übersichtlichkeit ist es auch sinnvoll, kleine verständliche Einheiten zu packen. Die Komplexität wird damit aber nicht geringer, sondern veteilt sich nur auf mehr Einheiten. Und damit steigt dann auch der Aufwand, alles unter einen Hut zu bekommen. Das ist dann Abwägungssache, was einem mehr Vorteile bingt.

Mein Wunsch und Ziel ist es auch nach Monaten Abstand von einem Projekt schnell zu erkennen, was es kann, welche "Legosteine" (Klassen, Funktionen) ich zur Verfügung habe, welche ich zusammengebaut habe und ob ich einen Legostein anpassen/erweitern muss oder neuen brauche.

Da hilft vor allem ein gutes Ordnungssystem und aussagekräftige Bezeichnungen. Es ist zum Beispiel wenig sinnvoll, alle Interfaces in ein Verzeichnis zu packen, nur weil es Interfaces sind. Sinnvoller empfinde zumindest ich es, die Einheiten nach fachlichen Kriterien zusammenzufassen. Dann kann man schon anhand der Verzeichnisstruktur sehen, wie das Projekt gegliedert ist, und was zu wem gehört.

dedlfix.

[Rolf B]

Hallo ebody,

ich hätte einen Haufen Anmerkungen

Welches Modellierungstool, bzw. welche graphische Modellierungssprache ist das? In meinem UML Vokabular kommt ein ausgefüllter Pfeil im Klassendiagramm nicht vor.

In welcher Beziehung steht ListSheetData zu ReadSheet? Ist das eine Assoziation? Eine Subklasse?

Wenn es eine Assoziation ist - wie navigiert man vom einen zum anderen? Da sehe ich kein zugehöriges Property. Wenn ListSheetData ein Rückgabewert einer ReadSheet-Methode ist, kann man das im Modell darstellen? Ein Pfeil gehört da dann nicht hin, soweit ich weiß.

Klassen sollten in ihrem Namen keine Tätigkeit darstellen. Sie repräsentieren Dinge, keine Aufgaben. ReadSheet könnte sein Leben als SheetReader führen, in dem Fall müsste es zum SheetReader aber auf jeden Fall noch eine Klasse DataSheet geben. Ist das dein ListSheetData?

Wichtig ist auf jeden Fall, dass der SheetReader sich nur mit dem Lesen beschäftigt und sonst nichts. "Do One Thing And Do It Right".

Methoden wie sheetCheckUrl und setSheetUrl sind da schon verdächtig. Wenn der SheetReader Anforderungen an die URL hat, die über formale Ansprüche an eine URL hinausgehen (z.B. wenn Du nur URLs akzeptieren willst, die das Präfix https://docs.google.com/spreadsheets haben) bestimmte Hostnamen akzeptieren willst), dann sollte er sie beim Entgegennehmen der URL prüfen und eine Exception werfen, wenn es nicht passt. Eine explizite Check-Methode scheint unpassend. Eine set-Methode aber eigentlich auch, die URL kannst Du auch an den Konstruktor übergeben. Andere Fehler, wie einen Existenzcheck der URL, würde ich an der Stelle lassen, weil sie einen Netzzugriff brauchen und daher asynchron sind. Das führt zu einem HTTP Fehlercode beim Lesen, das ist früh genug. Oder Du erstellst eine checkExists Methode, wenn Du wirklich nicht mehr tun willst als die Existenz zu testen.

Die normalen Anforderungen an die URL sollten aber durch die URL-Klasse geprüft werden, die der Browser (außer Internet Explorer) mitbringt. Wenn Du den IE unterstützen willst, musst Du Dir einen URL-Polyfill suchen - aber ich würde die URL Validierung aus dem Reader heraushalten.

Methoden wie countRows() und countCols() sollten demnach Propertys des DataSheet sein, und die SheetUrl sollte der Reader per Konstruktor bekommen. Ist sie falsch - wie gesagt: Exception.

countRows und countCols sind ebenfalls falsch benannt. „count“ sieht hier nach einem Verb aus, und ein Verb gehört zu einer Methode. Aber das sind augenscheinlich keine Methoden, sondern Property-Getter, die den Wert liefern, und demnach sollten sie rowCount und columnCount heißen.

Ein Getter namens transformSheet scheint mir auch verdächtig. Er sollte transformedSheet heißen, wenn er das transformierte Sheet liefert. Alternativ müsste es eine Methode transformSheet() sein, die die Transformation ausführt.

Frage: Wie löst Du die Asynchronität des Lesevorgangs? Ein Datenzugriff über eine URL dauert eine Weile, das ist in JS prinzipiell asynchron.

Wenn ich das baute, würde ich eine read-Methode vorsehen, die ein Promise zurückliefert, so dass man als Anwender dies tun kann:

new SheetReader("http://example.com/somesheet")
.read()
.then(sheet => {
   // sheet verarbeiten
})
.catch(err => {
   // Fehlerbehandlung
});

oder mit neuerer JavaScript Syntax, in einer async Funktion:

try {
   let sheet = await new SheetReader("...").read();
   // sheet verarbeiten
}
catch(err) {
   // handle error
}

Innerhalb von read() kannst Du das mit dem fetch-API implementieren, das ist ohnehin schon Promise-basierend, oder Du kapselst den XmlHttpRequest selbst in ein Promise. Ach, IE. Da bräuchtest Du dann auch einen Polyfill für Promises. Oder Du verzichtest auf IE User, wenn Du Dir das erlauben kannst.

Rolf

[ebody]

Vielen Dank schon mal euch beiden. Das hilft mir schon mal weiter.

Das sollte ein UML Klassendiagramm sein, eine grobe Übersicht für mich, aber muss mich nochmal damit befassen, um korrekte, logisch nachvollziehbare Diagramme zu erstellen.

Ich habe alles nochmal überdacht und jetzt eine solche Klasse (ja nicht korrekt, als grober Entwurf gedacht) erstellt.

Von getData() ist alles abhängig. getData() soll direkt mit Aufruf der Klasse ausgeführt werden (im Constructor). Hier wird geprüft, ob es sich um eine Google Spreadsheet URL handelt, die die Spreadsheet Daten als JSON zurückgibt.

https://spreadsheets.google.com/feeds/cells/.../1/public/full?alt=json

Und bei Erfolg, aus diesem JSON nur die Datensätze (inkl. Spaltenüberschriften) in einem Array speichert...Die anderen Methoden wie z.B. list(), welche das Array, die Datensätze Zeile für Zeile ausgeben soll, sollte nur dann ausgeführt werden können, wenn getData() Daten liefern kann.

Hier überlege ich gerade wie ich dass am besten nur mit der Klasse umsetze.

Ich könnte ein Script außerhalb der Klasse wie dieses verwenden...

if(new SpreadsheetData(spreadhsheetUrl) === success){
SpreadsheetData.list();
}

Oder

try {
   let sheet = await new SpreadsheetData(spreadhsheetUrl);
   SpreadsheetData.list();
}
catch(err) {
   // handle error
}

...aber würde es lieber so handhaben, dass wenn ich z.B.

SpreadsheetData.list();

anwende, ein Feedback bekomme, wenn es keine Daten gibt, die Funktion daher nicht nutzen kann, warum nicht und was zu tun ist.

Habt ihr evtl. einen Link zu einem Beispielscript, wie man so etwas am besten umsetzt oder wo so ein Vorgehen erläutert wird?

Gruß ebody

[Rolf B]

Hallo ebody,

mach nicht zu viel an einer Stelle. Du kannst die Datenbeschaffung im Konstruktor anstoßen, aber nicht durchführen, weil sie asynchron laufen muss.

class SpreadsheetReader {
   constructor(url) {
      // validiere url

      const fetchPromise = fetch(url, options)
                .then(response => {
                   if (!response.ok)
                      throw new Error(`HTTP error! status: ${response.status}`);
                   return response.json()
                })
                .then(json => new SpreadsheetData(json));

      Object.defineProperty(this, "complete", { value: fetchPromise, writable:false; });
   }
}

// Verwendung
const data = await (new SpreadsheetReader("https://google/bla/blub?alt=json")).complete;

// oder
(new SpreadsheetReader("https://google/bla/blub?alt=json"))
.complete.then(data => {
   // Verarbeiten
});

Der Konstruktor löst den Fetch-Request aus und legt das Promise für die Antwort in ein Property, das als readonly angelegt wird. Das ist die bessere Alternative zu einer getter-Methode, wenn Du keinen privaten Schreibzugriff brauchst.

Beachte, dass der await nicht auf das Konstruktorergebnis zielt, sondern auf das complete-Property, wo das Promise drinsteckt. Wenn das Ganze nicht in einer async-Funktion steckt, ist ein .then-Callback vermutlich einfacher als await.

Die Übergabe von json an den SpreadsheetData-Konstruktor könnte man weiter aufteilen. Man könnte durchaus sagen, dass die Konvertierung eines json-Strings in ein SpreadsheetData-Objekt nicht nach SpreadsheetData gehört. Und auch nicht in die Datenbeschaffung. Dafür könnte man eine Klasse SpreadsheetDataBuilder schreiben, die das json bekommt und ein SpreadsheetData Objekt liefert. Ihre Arbeit kann sie auf etliche kleine Methoden verteilen, die man dann separat mit Unit Tests absichern kann. Guck nicht so groß. Du fragtest nach OOP Best Practices. Kleinteilige Klassen und Unit Tests gehören dazu.

Ob Du den Reader unbedingt als Klasse implementieren musst, ist fraglich. Das Ding tut seinen Job nur im Konstruktor vermutlich könntest Du den auch als Funktion readGoogleSpreadsheet(url) implementieren. Entweder global, oder als Methode irgendeiner geeigneten Klasse.

Wenn Du unbedingt jquery-Ajax verwenden willst, dann kannst Du immer noch mit Promises arbeiten. $.ajax() liefert ein jQuery-Deferred Objekt zurück, das sich wie ein Promise verhält und demzufolge auch zu await kompatibel sein sollte.

$.ajax({ url: '...', type: 'json', ... })
.then(jsondata => (new SpreadsheetDataBuilder()).buildFromJson(jsondata))

Die Kleinteiligkeit schafft nicht unbedingt bessere Lesbarkeit. Da muss man gut aufpassen. Was sie schafft, ist Testbarkeit. Je kleiner die Bausteine, desto besser kannst Du jeden für sich in Isolation testen.

Robert C. Martin - Clean Code

Rolf

[dedlfix]

Tach!

Das sollte ein UML Klassendiagramm sein, eine grobe Übersicht für mich, aber muss mich nochmal damit befassen, um korrekte, logisch nachvollziehbare Diagramme zu erstellen.

Erstmal geht es darum, eine Struktur in dein Projekt zu bringen. Bevor du über Klassen und Diagramme dazu nachdenkst, steht erstmal an, die Zuständigkeiten zu klären. Als Beispiel: Eine Kuh melkt sich nicht selbst. Das macht der Bauer oder eine Melkmaschine. Die Milch ist auch nicht diejenige, die Butter und Käse aus sich macht. Auch die Kuh macht das nicht, sondern andere Spezialisten sind für diese Weiterverarbeitung zuständig.

Die Spreadsheet-Daten sollten lediglich die Daten selbst sein. Beschaffen ist eine Geschichte, Weiterverarbeiten eine andere. Die Datenbeschaffung muss Dinge mit dem System regeln, über das sie die Daten holt. Zum Beispiel einen Querystring zusammensetzen oder den Body für einen POST-Request, die Response entgegennehmen und gegebenenfalls aus einem JSON-String ein Objekt wiederherstellen. Auch das Reagieren auf Fehlerfälle gehört dazu. Das wird allein schon umfangreich genug, dass man dazu nicht auch noch tausend andere Aufgaben in die Klasse packen sollte, auch wenn sich nach außen hin alles hinter einer einzelnen Methode versteckt und damit auf den ersten Blick klein genug aussieht.

Filtern und suchen (wo ist da der Unterschied?) und andere Dinge, die die Daten weiterverarbeiten, sind besser in einem eigenen Service abseits der Beschaffung aufgehoben. Ob diese Aufgaben jeweils einen eigenen Service bekommen sollten oder alle in einem gut aufgehoben sind, ist Ansichtssache. Das hängt vor allem davon ab, wie umfangreich die jeweiligen Aufgaben sind. Refakturieren kann man später immer noch.

Von getData() ist alles abhängig. getData() soll direkt mit Aufruf der Klasse ausgeführt werden (im Constructor). Hier wird geprüft, ob es sich um eine Google Spreadsheet URL handelt, die die Spreadsheet Daten als JSON zurückgibt.

Lieber nicht. Man könnte zwar das Promise im Konstruktor erstellen, aber Promises laufen sofort los, wenn man sie erstellt, nicht erst, wenn jemand kommt und sich für das Ergebnis interessiert. Im Zweifelsfall wird also das System gleich beim Startvorgang mit zu dem Zeitpunkt unnötigen Dingen beauftragt. Das Promise würde ich erst dann erstellen, wenn der erste Verwender es haben möchte.

dedlfix.