Functionsparameter prüfen und eine Exception werfen !?

[perlenfischer1984]

Hallo zusammen ich habe mal eine generelle Frage :
Ich schreibe ein größere Anwendung und bin dabei in allen öffentlichen und teils auch in den privaten Methoden eine Parameterprüfung zu entwicklen.
Das sieht dann so aus

    // throw an exception if the text is empty or null
     if (url == null || url.isEmpty()) {
       throw new WebContentReaderException("can't parse the text, because the text parameter is empty or null");
     }

     // throw an exception if the patternList is null or empty
     if (patternList == null || patternList.isEmpty()) {
       throw new WebContentReaderException("can't parse the text, because the text parameter is empty or null");
     }

     // throw an exception if the matcherGroupIndex < 0
     if (matcherGroupIndex < 0) {
       throw new WebContentReaderException("can't parse the text, because the text parameter is empty or null");
     }

Da ich für jede Funktion auch einen Unit Test schreibe, bläht es den Code enorm auf. nun meine Frage ist es überhaupt sinnvoll, jeden Parameter gegen einen Null - Check zu prüfen oder einfach dann mit einem default wert weitermachen - wenn es sinnvoll ist?

Vielen Dank für eure Hilfe

 

[thecain]

Wenn es sinnvoll ist spricht nichts dagegen, aber einen Test müsstest du dann ja trotzdem schreiben 😉 Was aber nicht schlimm ist

 

[perlenfischer1984]

Danke für deine schnelle Antwort. Ich habe eine Klasse mit einigen Funktionen und in jeder prüfe ich ob der String nicht null oder leer ist. Und mit den anderen PArameter ähnlich. Das ist dass was ich meine, das es dann eher unleserlich wird, da der Code so lang ist.

Wie macht ihr das

 

[mrBrown]

Ich nutz aktuell dafür Springs Assert-Klasse, sowas lässt sich auch leicht selbst schreiben.
Im Prinzip also Auslagern der if's in eine Methoden, macht den Code kompakter, leserlicher und mMn auch Fehlerunanfälliger.
In deinem Fall gäb's dann eine Assert.notEmpty(String string), dir für null oder leeren String die WebContentReaderException schmeißen würde, und im Code stand dann nur noch Assert.notEmpty(url), statt dem if-Block.
Bei vielen checks nacheinander könnte man es auch so schreiben, dass man 'n Array aller Argumente übergibt, dann hätte man nur noch eine Zeile Assert.notEmpty(url,patternList).
Für den Check <0 dann sowas wie Assert.greaterThan(matcherGroupIndex,0) oder Assert.greaterThan0(matcherGroupIndex).

Die UnitTest sind davon ja nicht betroffen, da sollte man ja sowieso jeden Case abdecken. Ob man dann testet ob das ein valides Ergebnis ist, wenn man Defaultwerte nutzt, oder auf die Exception prüft, macht ja keinen großen Unterschied.

 

[kneitzel]

Also Parameter, die vom Design her gesetzt werden müssen werden geprüft und es wird eine Exception geworfen. Das ist aus meinen Augen sehe wichtig und wird bei unseren code analysis Tool auch geprüft. Ein Argument ist einfach schon, dass Du den Fehler direkt an der Stelle aufzeigen willst, an der er bekannt wird.

Wenn es sinnvolle defaultwerte gibt, dann werden die natürlich verwendet aber oft gibt es die eben nicht.

 

[Thallius]

Naja,

Ich würde da ml strickt trennen ob eine Methode eine Art API darstellt oder ob sie nur intern in diesem einen Project benutzt wird. Im ersten Fall sollte die Exception natürlich behandelt werden. Im zweiten Fall kann man theoretisch einen knallharten System.out.print machen und danach einen System.Exit(-1)
Denn ein Fehlerhafter Aufruf einer Methode kann nur durch einen fehlerhaften aufrufenden Code erzeugt werden. Und da ist es am einfachsten wenn man währende der Entwicklung direkt einen Cut macht und sieht wo es gekracht hat, als wenn man da irgendwelche Excpetions nach oben durchreicht.

Gruß

Claus

 

[mrBrown]

Naja,
Ich würde da ml strickt trennen ob eine Methode eine Art API darstellt oder ob sie nur intern in diesem einen Project benutzt wird. Im ersten Fall sollte die Exception natürlich behandelt werden. Im zweiten Fall kann man theoretisch einen knallharten System.out.print machen und danach einen System.Exit(-1)
Denn ein Fehlerhafter Aufruf einer Methode kann nur durch einen fehlerhaften aufrufenden Code erzeugt werden. Und da ist es am einfachsten wenn man währende der Entwicklung direkt einen Cut macht und sieht wo es gekracht hat, als wenn man da irgendwelche Excpetions nach oben durchreicht.

Solange man 'ne Unchecked Exception wirft, wird die im Normalfall nicht gefangen, falls man die wahllos fängt, liegt das Problem eher an anderer Stelle...
Zusätzlich zum Beenden hat man mit Exceptions auch noch 'nen Aussagekräftigen Stacktrace.

Wenn man nur System.Exit(-1) macht, hat man nur Ausgabe vom serr, aber darin auch keine Angabe woher der Aufruf kam. Außerdem macht man damit erstmal den Code untestbar, weil die ganze JVM beendet wird.

 

[thecain]

Ich stelle mir immer die Frage, warum kann der Wert null sein. null ist meiner Erfahrung nach zu 90% schlechtes Design. Wenn es sich im aktuellen Projektstand noch vermeiden lässt, würde ich da ansetzen, heisst, warum gibt es die Möglichkeit, dass null übergeben wird.

Ist es wirklich ein möglich, dass null übergeben wird, muss man darauf prüfen und die entsprechenden Exceptions werfen. Ob checked oder unchecked ist ein Diskussionspunkt und kommt auf die Umstände an, die ich hier nicht beurteilen kann. Wie man diese Checks dann macht ist grundsätzlich egal. Wichtig ist, dass es gut lesbar ist. Eine Methode wie validateXyz() welche die Prüfungen macht wäre etwa mein Ansatz.

 

[kneitzel]

Das Problem ist ja doch gerade, dass das Design kein null vorsieht, aber da eine Referenz benötigt wird bietet Java nun einmal auch null an.

Und das Validieren in eine eigene Funktion zu packen halte ich für ungeeignet. Das ist ja nur ein if ... throw new ...

Und da es ein Entwicklerfehler ist, ist es Unsinn, da eine checked Exception zu nehmen. Ich fange doch nicht ab, dass ich zu blöd war selbst zu sehen, dass ein Wert null sein konnte. Da prüfe ich doch lieber selbst auf null.

Aber das ist nur meine Sicht. Kann natürlich jeder selbst so machen wie er will.

 

[thecain]

Das Problem ist ja doch gerade, dass das Design kein null vorsieht, aber da eine Referenz benötigt wird bietet Java nun einmal auch null an.

Wenn ich eine API habe, dann biete ich möglichst keine Methode an, welche null erlaubt. Dafür kann ich Methoden ja überladen. Wenn dann jemand falsch aufruft ist es auch OK, wenn es knallt. Intern kann ich dann immer noch mit einem Optional arbeiten, wenn ich das will.

Und das Validieren in eine eigene Funktion zu packen halte ich für ungeeignet. Das ist ja nur ein if ... throw new ...

Zugegeben eigene Präferenzen hier. In der Hoffnung das diese 3 Parameter aus dem selben Objekt könnten (oder auch einzeln) hätte ich alle 3 oder mehr in einer Methode geprüft um die tatsächliche Logik nicht vollzumüllen.

Und da es ein Entwicklerfehler ist, ist es Unsinn, da eine checked Exception zu nehmen. Ich fange doch nicht ab, dass ich zu blöd war selbst zu sehen, dass ein Wert null sein konnte. Da prüfe ich doch lieber selbst auf null.

Wenn ich es beeinflussen kann (Codingfehler), dann stimm ich dir zu. Wenn ich Beispielsweise ein File lese, welches mir ein User zur Verfügung stellt, sieht es anders aus. Wobei ich auch zugeben muss, dass ich immer mehr ein Fan davon werde alle Exceptions unchecked zu haben, wie es in anderen Sprachen bereits üblich ist.

 

[kneitzel]

Wie erlaubst du kein null? Setzt du ein Contract Framework ein, so dass Du dies vorgeben kannst? An der Stelle habe ich im Augenblick Probleme, Deine Aussage richtig zu verstehen.

 

[thecain]

Zur Compilezeit mit einem @NotNull. Zur Runtime mit einer Exception. Nichts spektakuläres.

Ich wollte nur hervorheben, dass ich es für falsch halte ein Interface anzubieten, welches es erlaubt einen optionalen Parameter mit null anzugeben und dann einen Default Wert zu setzen.

Siehe:

oder einfach dann mit einem default wert weitermachen - wenn es sinnvoll ist?