Java Backend: Ist diese Methode zu komplex?

[CyborgBeta]

Schon gut. Hab mich vertan!

 

[tollertyp]

Also vernünftige Methoden mit vernünftigen Namen machen Kommentare mehr oder weniger unnötig. Ausnahmen bestätigen wie immer die Regel.

 

[CyborgBeta]

Ja, eigentlich sollten an jede Methode ja auch Java-Doc-Kommentare geschrieben werden. Aber dabei kann man auch viel verkehrt machen (weiß ich aus eigener Erfahrung).

 

[tollertyp]

An jede Methode? Nein.
Ich würde JavaDoc an Methoden machen, die als öffentliche Schnittstelle zu betrachten sind.

 

[BeBur]

Kommentare sollten sich auf das "Warum" konzentrieren, und nur in Ausnahmefällen auf das "Was".

Ich muss allerdings sagen, ich habe die "Abschnittkommentare" Der Kollegen zu schätzen gelernt. Besser wäre zuweilen eine eigene Funktion, aber wenn eine Funktion durch Kommentare in Abschnitte unterteilt wird, dann ist das schon viel Wert.

 

[tollertyp]

Wo ich Abschnitt-Kommentare übrigens sehr gut finde: Unit-Tests.

Auch wenn sie sich in jeder Methode wiederholen, zeigen was Vorbereitung ist, was der eigentliche Test-Aufruf, und was die Prüfungen sind. Führt meiner Meinung nach dazu, dass man bessere Tests schreibt, auch wenn man das auf den ersten Blick nicht sehen mag.

Und ja, je nach Klasse oder so können Abschnittskommentare auch mal sinnvoll sein. Wie bei jeder Regel gilt: Wenn du sie verstanden hast, dann brich sie, wenn du dafür gute Gründe hast.

Ich würde auch nicht sagen: Mach niemals bei einer privaten Methode eine JavaDoc hin.

 

[maxpayne80]

1. Lieber mehr kurze Methoden als einen Riesenblock
2. Sprechende Variablennamen ( und Methodennamen für 1. )
und :
3. Lieber eine eigene Klasse bauen, anstatt x-Fach verschachtelte Listen ( hab sowas wie in deinem OP schon lange nicht mehr gesehen ... grenzt doch an Masochismus )

Gerade letzteres ... glaub mir, nicht mal Du selbst verstehst den Code noch, wenn Du nach einem Jahr mal wieder draufschaust.

 

[tollertyp]

@maxpayne80: Hört sich ziemlich nach Vorschlägen aus dem Buch Clean Code an - meine jetzt nicht, dass du sie kopiert hättest oder so, ich will damit eher sagen, worum es in dem Buch überwiegend geht...

 

[CyborgBeta]

Lieber mehr kurze Methoden als einen Riesenblock

Das ist halt immer pauschal gesagt ... ab wann ist eine Methode zu lang, wann übernimmt sie zu viele Aufgaben oder Zuständigkeiten, ab wann ist kein logischer Zusammenhang mehr da?

Sprechende Variablennamen ( und Methodennamen für 1. )

Die Variablennamen sind schon sprechend. Es ist zudem legitim, temporäre Blockvariablen mit kleiner Sichtbarkeit abzukürzen (im Englischen nennt man diese throwaway variables).

Lieber eine eigene Klasse bauen,

Auch da bin ich zwiegespalten. Es gibt die KISS, YAGNI und Overengineering Pattern/Prinzipien. Oder anders: Im Extremfall pflastert man alles mit extra Klassen zu, wobei viele dieselbe Zuständigkeit übernehmen. Oder auf Deutsch: Viele Köche verderben den Brei.

 

[tollertyp]

Wenn man Spaghetti mag, dann lieber weniger Köche.
Und ja, selbst vergebene Bezeichner sind immer vielsagend. Für den Moment.

Und ja, es spricht nichts dagegen, bei einer einfachen for-Schleife i zu verwenden... aber bei dir sprechen wir nicht mehr von einer einfachen for-Schleife. Dein Code verstößt gegen den allgemeinen Grundsatz, der eher aus dem Design kommt: "Don't make me think".
Also "zwing mich nicht dazu, nachdenken zu müssen". Dein Code, der in einer for-Schleife eine Laufvariable definiert, die aber selbst ausschließlich in einer inneren While-Schleife inkrementiert wird, das ist schon Kunst.

 

[CyborgBeta]

ja, selbst vergebene Bezeichner sind immer vielsagend. Für den Moment.

Das stimmt so nicht. Die Variablennamen sind in Ordnung. Es geht nicht darum, diese künstlich in die Länge zu ziehen.

Du würdest auch nicht "Prinz Ernst August Albert Paul Otto Rupprecht Oskar Berthold Friedrich-Ferdinand Christian-Ludwig von Hannover" schreiben, wenn es auch einfach "Prinz August" täte ...

 

[tollertyp]

Du machst dir echt Sorgen um künstliches in die Länge ziehen bei solchen Statements?

while (isFirstEntry || i < data.size() && data.get(i - 1).get(0).equals(data.get(i).get(0))) {

 

[CyborgBeta]

Alles legitim aus meiner Sicht. Aber lassen wir das. "Richtige" Variablennamen finden, ist zum Teil auch subjektiv. Oder anders: Man muss immer zwischen ausreichend Verständlichkeit und "mental clutter" abwägen.

Ergänzung (18. Oktober 2024)

Edit: Und nein, für 0 und 1 führe ich keine extra Variablen ein.

 

[tollertyp]

Das geht ja auch eher in Richtung Kritik an dieses Statement, weniger an das i selbst...
Wäre das Statement lesbarer, wäre das i auch harmloser.

Ja, man kann schon erkennen, was das macht. Aber das sollte nicht der Maßstab für guten Code sein. Und das hat sich der Reviewer halt vermutlich auch gedacht.
Und es ist halt nicht so, dass es nur eine Stelle wäre, die einen zum Nachdenken bringt. Es ist die Summe. Wenn es nur dieses While wäre, was unschön ist, würde ich auch sagen "kann man schöner machen, aber drauf geschissen..."

 

[CyborgBeta]

Edit 2: Falls es um || und && geht ... das ist schon die K DNF, also auch in Ordnung.

Ergänzung (18. Oktober 2024)

Edit ... Ja, ok, also zumindest mit einigen Kommentaren beschreiben, was die drei Schleifen machen.

 

[maxpayne80]

Das ist halt immer pauschal gesagt ... ab wann ist eine Methode zu lang, wann übernimmt sie zu viele Aufgaben oder Zuständigkeiten, ab wann ist kein logischer Zusammenhang mehr da?


Die Variablennamen sind schon sprechend. Es ist zudem legitim, temporäre Blockvariablen mit kleiner Sichtbarkeit abzukürzen (im Englischen nennt man diese throwaway variables).


Auch da bin ich zwiegespalten. Es gibt die KISS, YAGNI und Overengineering Pattern/Prinzipien. Oder anders: Im Extremfall pflastert man alles mit extra Klassen zu, wobei viele dieselbe Zuständigkeit übernehmen. Oder auf Deutsch: Viele Köche verderben den Brei.

Ich bin kein Verfechter irgendeines der vielfältigen Vorschläge, wie man seinen Code gestalten kann ( kann quasi religiöse Züge annehmen, habe auch Fans von Onkel Bob in der Firma ... ).

Nein, was ich geschrieben habe ist tatsächlich einfach "spontan", was mir nach so paar Jahren im Java-Umfeld hier und da aus eigener Erfahrung als hilfreich / weniger hilfreich über den Bildschirm gewandert ist.

Soll jeder dem Konzept folgen, was ihm beliebt. Ich nehme mir die Teile, die sich als tauglich erweisen.
Dazu gehört auch, dass ich mich aus religiösen Konversationen heraushalte, Energieverschwendung.

Was Dir aber jeder sagen wird, ganz gleich der eigenen Vorlieben oder Vorgaben im Job :
diese Methode in Post #1 ist gleich in mehrfacher Hinsicht schlecht.

Ob Du auf vielfach gute Tipps, die ich hier gesehen habe, eingehen willst - Deine Sache. Kommt mir bisschen Stur vor Deine Einstellung ( no offence ).

Bin dann mal raus, viel Erfolg wünsche ich Dir :-)

 

[tollertyp]

Ja, ok, also zumindest mit einigen Kommentaren beschreiben, was die drei Schleifen machen.

Und genau der Satz drückt deine Haltung zu gutem Code eben aus. Und das ist kein Lob.

Ich versuche jeden Kommentar zu vermeiden? Warum? Kommentare sind meistens Redundanz. Ich hasse Redundanz, außer es geht um Ausfallsicherheit/Backups.

Ein Kommentar, der beschreibt, was der Code macht, ist schlicht redundant. Und was passiert, wenn der Code angepasst wird? Der Code wird geändert, der Kommentar bleibt oft genug gleich...

Und ich zitiere mich einfach mal selbst:

Kommentare sollten sich auf das "Warum" konzentrieren, und nur in Ausnahmefällen auf das "Was". Letzteres ist, wie du sagst, ein Zeichen für schlechten Code. Der Code sollte selbst ausdrücken, was er macht. Und dafür ist der Eröffnungspost ein gutes Beispiel, wie man es nicht machen sollte.

 

[CyborgBeta]

Noch mal zurück zu #52 ...

1. Ich meinte DNF, nicht KDNF oder (full DNF...), hab das schon verbessert(1),
2. für mich ist das noch "lesbar"... aber wenn es tatsächlich zu kompliziert ist, dann sollte die if-Bedingung in einer eigenen Methode stehen.

(1) interessantes Video dazu:

YouTube

An dieser Stelle steht ein externer Inhalt von YouTube, der den Forumbeitrag ergänzt. Er kann mit einem Klick geladen und auch wieder ausgeblendet werden.

YouTube-Embeds laden

Ich bin damit einverstanden, dass YouTube-Embeds geladen werden. Dabei können personen­bezogene Daten an YouTube übermittelt werden. Mehr dazu in der Datenschutzerklärung.

YouTube-Embeds laden

Datenschutzerklärung

 

[tollertyp]

für mich wäre das dennoch lesbarer:

while (i < data.length() && isRowOfSameTable(data, i)) {
...

private static boolean isRowOfSameTable(final List<List<Object>> data, final int rowIndex) {
  return (rowIndex > 0) && Objects.equals(data.get(rowIndex - 1).get(0), data.get(rowIndex).get(0));
}

ungetestet selbstverständlich

Tatsächlich würde ich aber den ganzen Algorithmus ändern:

    // die beiden Zeilen würde ich in eine eigene Methode auslagern
    List<List<Object>> data = MDB.get();
    data.sort(Comparator.comparing(a -> ((java.sql.Timestamp) a.get(0))));
    if (data.isEmpty()) {
      return Collections.emptyList();
    }

    List<List<List<Object>>> tables = new ArrayList<>();
    List<List<Object>> currentTable = new ArrayList<>();
    Object currentTimestamp = null;
    tables.add(currentTable);
    for (int i = 0; i < data.size(); i++) {
      final List<Object> currentRow = data.get(i);
      if (!Objects.equals(currentTimestamp, currentRow.get(0))) {
        currentTable = new ArrayList<>();
        tables.add(currentTable);
        currentTimestamp = currentRow.get(0);
      }
      currentTable.add(currentRow);
    }

aber je mehr ich drüber nachdenke, desto eher würde ich in Richtung Stream gehen und groupingBy nutzen, um alles in eine Map<Object, List<List<Object>>> zu bekommen, wobei das erste Object dann der Timestamp zu der Map wäre:

Map<Object, List<List<Object>>> tablesMap = data.stream()
    .collect(Collectors.groupingBy(row -> row.get(0)));

Man kann, wenn man dann das ganze wieder in eine Liste bringen:

List<List<List<Object>>> tables = new ArrayList<>(tablesMap.values());

die dann natürlich noch sortiert werden müsste, die eingelesenen Daten selbst müssten es nicht.

Edit: hat jemand eine Ahnung, warum, wenn ich Sachen in Code-Blöcken kopiere, immer so ein "Müll" erst mal kopiert wird?

Ich hab den Code kopiert und das kommt raus, wenn ich es in Firefox einfüge, wenn ich den Umweg über einen Editor wie VS Code gehe, passt alles:

while (isFirstEntry || i &lt; data.size() &amp;&amp; data.get(i - 1).get(0).equals(data.get(i).get(0))) {

 

[CyborgBeta]

hat jemand eine Ahnung, warum, wenn ich Sachen in Code-Blöcken kopiere, immer so ein "Müll" erst mal kopiert wird?

Das ist meines Erachtens ein Bug in der Forensoftware bei der Funktion zum Zitieren von Code ... Ein Fall für @Steffen vielleicht