Doppelte checks vermeiden?!

[tuxedo]

Hallo,

ich hab da eine kleine Design-Frage. Weiß noch nicht so recht wie ich das ganze benennen soll. Ich mach am besten mal ein Beispiel:

Angenommen ich habe eine ZIP-Datei die eine bestimmte Struktur enthält, bzw. enthalten muss. Und ich hab eine Klasse, die als Container für solche Zip-Dateien gilt.

Diese Klasse könnte z.B. so aussehen:

public class Archive
{
    
    public Archive(File f)
    {
        if (!isArchive(f))
            throw new InvalidArchiveException("foobar");
        // .....
    }
    
    public static boolean isAccepted(File f) {
        // prüft ob die angegebene Datei wirklich ein "Archive" ist wie es erwartet wird
    }
    
    // ... diverse getter folgen ...
    
}

Nun hab ich ein Programmteil der ein Verzeichnis durchschaut und die einzelnen Files auf "ist ein gültiges Archiv" prüft. Dazu würde ich die "isAccepted()" Methode aufrufen.
Wenn da "true" zurück kommt weiß ich dass es okay ist die File in den Konstruktor zu stecken und eine Instanz anzulegen.
Aber prinzipiell muss ich ja auch dort prüfen ob die File gültig ist.

Entweder lasse ich das vorherige, einzelne Abfragen auf "isAcppected()" weg und verlasse mich auch eine Exception, oder ich akzeptiere dass ich die ZIP-File zweimal anschauen muss.

Das Beispiel sollte man eher abstrakt und suboptimal zu betrachten. Aber ich denke ich versteht worauf ich hinaus will, oder?

Wie handhabt ihr sowas? Prüft ihr dann einfach zweimal? Oder lasst ihr den Programmfluss dann einfach in die Exception laufen?
Oder geht ihr gar ganz andere Wege?

Gruß
Alex

 

[SlaterB]

isArchive() == isAccepted()?

denkbar wären noch verschiedene Konstruktoren, in einem versichert der Aufrufer, dass er es gerade geprüft hat, dann kann man auf eine neue verzichten,
oder nur ein Konstruktor mit boolean true/ false, wo jeder Aufrufer dazu Stellung nehmen muss,
höhere Komplexität für gesparte Prüfung

 

[Tobias]

Das hängt - wie immer - vom Kontext ab. In diesem speziellen Fall würde ich nicht doppelt prüfen, sondern im Verzeichnis-Crawler die Datei einfach in den Konstruktor stecken ohne vorher explizit zu prüfen, wenn ich vom Benutzer erwarte, das er in seinem Verzeichnis nur gültige Archive aufbewahrt. Kommt dann eine Exception, würde ich eine Meldung ausgeben und fertig.

Wenn es dagegen legal ist, im Verzeichnis auch andere Dateien liegen zu haben, würde es sich ja nicht mehr um eine Ausnahme handeln, weshalb oben skizzierter Kontrollfluss nicht korrekt wäre. Dann würde ich doppelt prüfen.

Auf die doppelte Prüfung verzichten könnte man, wenn man die Prüfung im Konstruktor wegläßt (denkbar etwa, wenn an anderer Stelle eine Exception auftreten wird, weil das Archiv nicht die gewünschte Struktur hat). Das müßte man dann aber explizit dokumentieren, weil die Exception dann wahrscheinlich nicht mehr so aussagekräftig wäre wie eine InvalidArchiveException.

 

[tuxedo]

@Slater

isArchive() == isAccepted()?

Jepp. Wie gesagt: Suboptimales beispiel

denkbar wären noch verschiedene Konstruktoren, in einem versichert der Aufrufer, dass er es gerade geprüft hat, dann kann man auf eine neue verzichten,
oder nur ein Konstruktor mit boolean true/ false, wo jeder Aufrufer dazu Stellung nehmen muss,
höhere Komplexität für gesparte Prüfung

Im Falle einer API wär das ne gute Idee. Aber was, wenn es keine API ist und es mehrere Archivarten geben kann?
Hmm, *Idee*...
Dann bräuchte man etwas das rausfindet was es für eine Archivart (erste Prüfung) ist und dementsprechend handelt (die entsprechende Archiv-Klasse zur Instanziierung hernehmen, wo dann im Konstruktur nochmal geprüft wird?). Dann hätte man zwar im einzelnen wieder doppelt geprüft, aber man müsste nicht für jede Art jeweils doppelt prüfen. UND die erste Prüfung wäre die maßgebende, die zweite wäre dann in der Tat eine Ausnahmebehandlung, für den Fall dass jemand direkt diese Eine Archivart instanziieren will und den falschen Typ hinein füttert.

- Alex

 

[Marco13]

Ja, das ist ziemlich genau das, was ich mal unter http://www.java-forum.org/allgemeine-java-themen/92447-performance-methodenaufruf-if-abfrage.html#post586045 geschrieben hatte. Wie da auch schon gesagt:
Das (selbstverständlich) wichtigste ist, dass das Verhalten dokumentiert ist.

Eine pauschale Aussage dazu ist schwierig. Wenn das ganze eine Bibliothek/API sein soll, sind mehrfache Tests natürlich "sicherer". Zumal man davon ausgehen kann, dass alles, was mit einer "File" zu tun hat, ohnehin so langsam ist, dass ein paar zusätzliche Überprüfungen keinen Unterschied machen. Es hängt aber IMHO auch davon ab, was passiert, wenn die Datei KEIN Archiv ist: Wenn das bewirken würde, dass viele verschiedene "unspezifische" Exceptions geworfen werden, und der Konstruktor dann

public Archive(File f) throws IOException, ZipException, FooException, SQLException, NoArchiveException, EndOfStreamException

heißen müßte, es es eigentlich klar: Man überprüft, und schmeißt die eine Exception, die alle anderen verhindert.

Im konkreten Fall könnte man auch überlegen, ob man eine package-private Methode verwendet, um die Archive für Files zu erstellen, die man schon überprüft hat, aber ... es war ja eine allgemeine Frage...