Pleiten, Pech und Pannen - oder die eigene Dummheit

Ich weiß nicht, ob wir so einen Thread schon irgendwo haben, aber ich wollte mal einen gerade behobenen Fehler der Klasse “Das kann gar nicht sein!” beschreiben:

Folgender Fehlerfall ist kurz vor der Auslieferung einer großen Datenmenge (mehrere Millionen Datensätze) aufgetreten: Es wurde festgestellt, dass in der Gesamtmenge eine bestimmter Ausprägung nur genau einmal vorkäme, obwohl dies mehrere hunderttausend mal vorkommen müsste.

Ich schaute in mein Programm, ich schaute in meine Testmenge. Da waren reichlich dieser Ausprägungen im Ergebnis vorhanden. Eine längere Suche nach einem dieser Datensätze in der großen Datei förderte folgendes Bild zutage:

Es gab Unterschiede zwischen den gleichen Datensätzen in der gesamten Auslieferung und der Testdatei!

Nach langen, langen Tests, die nur mit System.out.println(...) durchgeführt werden konnten, weil Debuggen auf der Gesamtmenge (auch wenn diese in Abschnitten á 500.000 Datensätzen vorlagen) nicht erfolgen konnte, fand ich den Fehler tief in einer der vielen Analyseklassen in den folgenden Zeilen:

    private static final MyClass STANDRDBELEGUNG_1 = new MyClass(
            "Parameter 1 a", "Parameter 2 a");
    private static final MyClass STANDRDBELEGUNG_2 = new MyClass(
            "Parameter 1 b", "Parameter 2 b");
    private static final MyClass STANDRDBELEGUNG_3 = new MyClass(
            "Parameter 1 c", "Parameter 2 c");

Was können diese harmlose Zeilen denn schlimmes bewirken?

Nun, es kommen im Code irgendwo Abschnitte der Art

    if (bedingung1) {
        myClass = STANDRDBELEGUNG_1;
    }

vor. Was ist daran schlimm? Noch gar nichts. Als ich die obigen Zeilen mit den Standardbelegungsobjekten hinschrieb war die Klasse MyClass noch immutual. Aber später erwies sich, dass eigentlich alle ihrer (mehr als hier gezeigten) fields veränderbar sein müssen. Also gab ich wieder mein schlechtes Bauchgefühl “klein bei”, entfernte das final vor jedem Feld und fügte Setter hinzu.
So weit so schlecht.

Im späteren Verlauf werden dann Dinge aufgerufen wie

    myClass.setParameter1("irgendwas anderes");

Was passiert nun? Da vorher myClass auf eine der statischen Vorbelegungen gesetzt wurde, wird hier nun eben diese Vorbelegung geändert. Und plötzlich stehen bei späteren Durchgängen irgendwelche Dinge in den Vorbelegungen, die dort nichts zu suchen haben!

Man hätte sich hier schlicht behelfen können, indem man das static entfernt. Aber ich bevorzugte den richtigen Weg zu gehen und MyClass wieder immutual zu machen. Dafür habe ich der Klasse Hilfsmethoden beigefügt, die ein passendes Objekt mit einem geänderten Wert zurückgeben.

Fazit:

Mutual objects are evil!

Und wie man sieht, kann es sogar ohne das Zusammenspiel mehrerer Threads zu wirklich unschönen Dingen kommen!

Du sagst du hast Tests, aber ohne assertions und stattdessen SysOuts?

Nicht alle Objekte können Immutable sein, wäre auch egal wenn sie nicht wiederverwendet würden, was sparst du denn durch static?

Nicht falsch verstehen als Kritik, sind nur Fragen ohne dass ich das Projekt kenne, Fehler machen gehört dazu.

Die Tests bezogen sich auf aktiv von mir durchgeführte Versuche, den Fehler zu finden. Dabei nutzte mir Debuggen auf der Testmenge herzlich wenig, weil auf dieser ja alles richtig lief (oder zumindest der Teil, den wir verglichen haben, weiter unten könnten durchaus auch Fehler auftauchen). Also blieb nur übrig, Ausgaben einzubauen, Jar bauen, auf Produktivrechner laufen lassen, Ausgaben vergleichen, von vorn. Und das halt ziemlich oft, bis ich die Quelle gefunden hatte.

Durch das static spare ich, das jedes Mal von den 500.000 die drei Objekte neu angelegt werden. Kein spürbarer Zeitverlust. Es wäre vermutlich eleganter gewesen, statt der statischen Vorlagen drei Methoden zu schrieben, die die Werte meines Objekts entsprechend setzen.

Da es sich hier aber umsetzen ließ, so dass das Datenobjekt immutal sein kann, habe ich aber diese Variante vorgezogen.

Man könnte die Wurzel des Übels auch in “premature optimization” sehen, diese statics zu vergeben. Ich habe unveränderliche Dinge gern als “Konstanten”, in Javafall eben static final, in meinem Code. Nur dass diese Dinge eben leider nicht unveränderlich waren.

wäre ein schönes (wenn auch vielleicht gefährliches, unpassendes) Sprachelement,
wenn man mit einer Variablen-Bezeichnung ähnlich final den inneren Aufbau eines bewußt normalerweise mutablen Objektes zum Zeitpunkt der Zuweisung des Objektes zur Variable festschreiben könnte,
alle direkten Attribute auch in Oberklassen für dieses Objekt (egal welcher möglichen Subklasse des Variablen-Types) wie final behandeln,

für Objekte wichtiger Systemklassen vielleicht mit Security-Sperre, Exception zum Zeitpunkt der Ausführung,
und wenn man Threads & Co. damit kaputt macht, ist erlaubt, dann auf eigene Verantwortung später die Exceptions bei der Arbeit mit diesem Objekt

gibt es dazu allgemeine Abhandlungen oder zu verrückt es genauer zu überlegen?
es braucht nicht nur ein eigenes javabasierstes Forum, es braucht ein eigenes javabasiertes Java! :wink:

edit:

bin ich gerade falsch oder benutzt du da das falsche Wort?

immutable, mutable evil?!

@SlaterB : das geht vielleicht in die Richtung: in “Java Concurrency in Practice” werden Annotationen verwendet, um den inneren Aufbau der Klasse zu beschreiben. (Findet man hier) Die Toolunterstützung ist dabei aber, soweit ich weiß, nicht sehr fortgeschritten. Ich kenne diesbezüglich nur die Warnungen, die man in IntelliJ IDEA aktivieren kann, wenn gegen die Annotationen verstoßen wird.

Ach so, dann waren das keine automatisierten Tests sondern debugging worauf sich deine SysOuts bezogen.

Nun, einer der groessten Aufwaende bei Tests sind oft die Testdaten zu erstellen, oft werden auch “syntetische” autom. Tests erstellt, also Szenarien gestestet, sie es so gar nicht gibt (speziell beim Tests, die halb integrationstests und halb Unit Tests sind, zB. wenn man Mock Obejkte in Integrationstests verwendet).

Refactoring ohne eine gute autom. Testsuit ist immer gefaehrlich, manchmal will man sich halt Aufwand ersparen… Multitheraded Tests sind auch moeglich, aber nicht immer einfach, Libs wie junit-toolbox helfen.

Verstaendlich, aber das ist wohl wirklich voreilige Optimierung, wenn man sich selber zwingt Performance Optimierungen durch Profiling nachzuweisen, fidnet man oft heraus, dass es keine Optimierungen sind.
Nebenbei, der Geltungsbereich von Variablen/Objekten beeinflusst das Verhalten des GC sehr, je kleiner umso besser, manchmal braucht es dann gar keinen GC lauf wenn zB klar ist dass bestimmte Variablen nur in einer Methoden “leben”.

Alles natuerlich nur dahingeredet ohne das konkrete Projekt und Problem zu kennen, ist immer einfach zu kritisieren wenn klar ist das es ein Problem gab, deswegen bitte nicht als Kritik auffassen, schon mutig ueber einen eigenen Bug eine Diskussion zu starten, soll nicht gegen dich arbeiten, im Gegenteil.

Hab erst vor ein paar Wochen Kollegen zeigen wollen wie man einen Build paralellisiert (so dass Maven die Module Parallel baut, einfacher als alle Tests parallel laufen zu lassen), dabei ist aufgefallen das eine Firmeninterne Lib nicht Threadsafe war (Excpetions flogen nur im Parallel Betrieb), es aber sein sollte, mit einem etwas geuebten Auge hat man gleich erkennen koennen dass es nicht Threadsafe war, GoF Singleton, static getInstance Methode aber nicht synchronized, stattdessen das INSTANCE Feld als volitaile deklariert, Konstruktoren oeffentlich(!!!), synchronisiert und prueften selber ob schon eine Instanz existierte… ich meine Angelika Langer hat daruber auch ein Beispiel gehabt mit volitaile und synchronized.
Design war also schlecht, nicht Threadsafe, keine Tests, voreilig optimiert, keine richtigen Reviews -> Probleme!
All das war jahrelang nicht aufgefallen…

Moral: Wenn es wichtig ist, sollte man Tests haben, auch wenn das manchmal mehr Aufwand bedeutet, zB. Multithreaded.
Refactoring ohne gute autom. Tests ist eben ein Spiel mit dem Feuer…

den Worst Case, dass 500.000x aufwendig der Speicher durchlaufen wird, muss man ja auch nicht annehmen,
eher sammeln sich kleine Reste, die vielleicht alle paar Tausend der 500.000 Durchläufe mit abgeräumt werden, schlimm genug

und selbst wenn ohne GC mit eigenen Befehlen punktgenau gearbeitet (wie in einem Daten-Array), hätte man da noch 500.000x Objekterstellungen, Speicherbelegung, Löschung,
falls statische Variable das vermeiden kann ist das schon verführerisch, in der Annahme dass es sonst keine Nachteile gibt

Enum ist der bekannte Fall, da könnte es mit den Bedingungen auch hingehen,
ohne das mutable-Problem zu lösen, ob direkt in Enum oder in MyClass-Objekten, in Enum vorgehalten

Verfuererisch auf jedenfall, aber Menschen sind sehr schlecht darin solch komplexen Ablaeufe nur im Kopf zu optimieren, neigen dazu dort zu „optimieren“, wo es gar keinen messbaren bzw. fuehlbaren Einfluss hat, das gilt auch dafuer, Nachteile abzuschaetzen.
Das muss man sich halt klarmachen…

Messen (Profiler) hat den Vorteil dass man konkrete Zahlen hat, wenn die Tests bzw. Testdaten die man misst aber allzu synthetisch bzw. Weltfremd sind, bringt auch das nix.

Wenn man allerdings einfachen Code schreibt, oder im Falle von Multithreading Code eher defensiv programmiert und dann mit echten Werten aus der Realitaet (oder eben guten Testsuites) optimiert wenn es noetig ist, ist man oft besser bedient.

Wenn Code nicht die richtigen Ergebnisse liefern muss, kann man ihn extrem schnell machen :wink:
Muss sich halt klarmachen was die Prioritaet hat.

*hab jetzt mangels Zeit nicht alles durchgelesen
@Crian
Der “Fehler” erinnert mich an einen Hinweis den ich mal bzgl T[].clone() bekommen habe. Es ging dabei um ein Event dass die geänderten Daten als Payload hat. Anstatt bei einem get() also das Array selbst zu returnen eben ein .clone() falls das eigentliche Array mehrfach verwendet wird (wenn mehrere Listener auf das Event reagieren). Sicher, wird durch ein Event ein Element des Arrays verändert betrifft diese Änderung dann auch alle anderen Zugriffe da halt keine deep-copy.

Wäre eine Zuweisung durch Setter / Konstruktor besser gewesen ? Möglich, aber man muss auch an den Overhead denken ob man 500k Objekte mit nur einem “kennzeichnet” oder die Daten 500k mal dupliziert. Dürfte aber auch nicht viel mehr ausmachen als wenn man Daten direkt als Member in die Klasse aufgenommen hätte.

Aber naja, passiert jedem mal, vor allem wenn nachträglich was verändert wird.

[QUOTE=SlaterB]wäre ein schönes (wenn auch vielleicht gefährliches, unpassendes) Sprachelement,
wenn man mit einer Variablen-Bezeichnung ähnlich final den inneren Aufbau eines bewußt normalerweise mutablen Objektes zum Zeitpunkt der Zuweisung des Objektes zur Variable festschreiben könnte,
alle direkten Attribute auch in Oberklassen für dieses Objekt (egal welcher möglichen Subklasse des Variablen-Types) wie final behandeln,

für Objekte wichtiger Systemklassen vielleicht mit Security-Sperre, Exception zum Zeitpunkt der Ausführung,
und wenn man Threads & Co. damit kaputt macht, ist erlaubt, dann auf eigene Verantwortung später die Exceptions bei der Arbeit mit diesem Objekt

gibt es dazu allgemeine Abhandlungen oder zu verrückt es genauer zu überlegen?
es braucht nicht nur ein eigenes javabasierstes Forum, es braucht ein eigenes javabasiertes Java! ;)[/QUOTE]

Stimmt, das wäre ziemlich schön, sowas zu haben.

[QUOTE=SlaterB;116855]bin ich gerade falsch oder benutzt du da das falsche Wort?

immutable, mutable evil?![/QUOTE]

ups. Stimt, ich hab mal leo befragt, mutual heißt “gegenseitig”, nicht “veränderbar”. schmunzelt Ich ändere es mal oben. Danke für den Hinweis, die falsche Verwendung von Fremdworten ist immer irgendwie peinlich…

*** Edit ***

Tja schade, ich kann es nicht mehr ändern. grinst

Aber mutable / immutable wäre richtig!

schmunzelt

FindBugs unterstuetzt @Immutable :slight_smile:

Nein nein, wenn ich eure Meinungen und Vorschläge nicht hören wollen würde, würde ich ja nicht hier meinen Fehler ausbreiten.
Mein Kollege, dem ich Anfangs das Problem beschrieb, meinte schon ganz richtig “Am Ende ist es sicher was, wo du dir an den Kopf fasst.”

Ich habe halt bestimmte Vorlieben, die solche Vorfälle - und Diskussionen hier - aber schonmal umstimmen können. Gerade den Punkt

finde ich sehr interessant. Im Grunde genommen bemühe ich mich im allgemeinen

  • mich an clean code zu halten
  • meine Sachen sinnvoll zu testen (wozu oft irgendwie die Zeit fehlt / es anders “erstmal” schneller geht/ etc. pp.)
  • Sachen erstmal einfach zu halten, also möglichst nichts voreilig zu optimieren.
  • Alles so unveränderlich wie möglich zu definieren, was geht
  • Ja auch ich bin ein Fan von kleinen Methoden mit sprechenden Namen. Namen sind mir eh sehr wichtig und ich bin froh, aus dem Zeitalter von getrennterm reinen Editor und Compiler (Linker, …) raus zu sein und mir von meiner IDE einfach überall einen Methodennamen ändern lassen zu können.

Nicht jeder Code erfüllt diese (und vergessene) Gesichtspunkte.

Davon ab bin ich nicht hier angemeldet, um mich zu profilieren. Sondern um dazu zu lernen, auch wenn ich schon “ewig und drei Tage” programmiere. Ich mach immer noch Fehler / unsaubere Dinge, also kann ich auch immer noch dazu lernen. Daher bin ich auch froh über Hinweise.

Genauer ins Detail dieses Projekts kann ich nicht gehen, da es ein Arbeitsprojekt ist.

Ich will auch endlich mal ein Projekt streng nach TDD umsetzen. Aber bisher obsiegte irgendwie immer meine Faulheit. Dabei weiß ich, dass man nur vordergründig Zeit spart, die man nachher beim Refakturieren draufzahlt oder es sogar riskiert, Projekte unwartbar werden zu lassen. Mmhmhm.

*** Edit ***

[quote=maki;116862]Moral: Wenn es wichtig ist, sollte man Tests haben, auch wenn das manchmal mehr Aufwand bedeutet, zB. Multithreaded.
Refactoring ohne gute autom. Tests ist eben ein Spiel mit dem Feuer…[/quote]

Ja. Da stime ich dir zu. Hier ist es im Prinzip so: Wenn die Ausgabedateien auf einer ausreichend großen Menge gleich bleiben, “glaube” ich der Refakturierung. Ich weiß, dass das nicht fein ist. Für die praktische Anwendung (dieses) Projekts reicht das allerdings aus, auch wenn es unbefriedigend bleibt.

[QUOTE=Crian][…]
Ja. Da stime ich dir zu. Hier ist es im Prinzip so: Wenn die Ausgabedateien auf einer ausreichend großen Menge gleich bleiben, “glaube” ich der Refakturierung. Ich weiß, dass das nicht fein ist. Für die praktische Anwendung (dieses) Projekts reicht das allerdings aus, auch wenn es unbefriedigend bleibt.[/QUOTE]

Ich bin immer noch der Meinung das ein Großteil von Unit-Tests unnötig ist wenn vermehrt Tools wie OpenJML zum Einsatz kämen. Ein prominentes Beispiel wie es in der Realität verwendet wird findet man hier Proving that Android. Und um auch mal zu sehen wie schlecht in großen Anwendungen getestet wird (die auch reichlich Unit-Tests haben) kann sich gerne mal der ein oder andere Blog-Beitrag unter https://aphyr.com/ zum Thema kaputtspielen von verteilten Systemen (z.B. MongoDB, Elastic Search) durchgelesen werden.

Und auch das Zusammenspiel von Threads könnte man vorab simulieren, PetriNetze fallen mir da ein oder Labeled Transition Systems (wie beschrieben in Concurrency, State Models & Java Programming) die als Modell dienen das dann in echten Code (meist per Hand oder wenn möglich maschinell) übersetzt wird.