✔ File auf Gültigkeit überprüfen

[y0dA]

Hi!
Also ich bekomme vom Benutzer eine Pfad-String sowie einen Filenamen-String und beide möchte ich überprüfen ob sie gültig sind.

Bsp:

public void isFileValid() throws NoFileException, FileNotFoundException {
		File file = new File(this.fPath + this.fName);
		if (!file.isFile()) {
			throw new NoFileException("blah");
		} else if (!file.exists()) {
			throw new FileNotFoundException("blah");
		} else if (!file.canRead()) {
			throw new CannotReadFileException("blah");
		}
	}

CannotReadFileException und NoFileException sind Spezialisierungen von IOException.

Nun meine Frage, macht das Sinn hier die Exceptions zu werfen oder sollte ich lieber wie folgt fortgehen:

public boolean isFileValid() {
		File file = new File(this.fPath + this.fName);
		return (file.isFile() && file.exists() && file.canRead());
	}

Ich möchte halt das Exception Handling richtig implementieren, nur finde ich die 1. Variante irgendwie komisch.
Weiters habe ich mit Variante 1. auch das "Problem" dass ich nun bis zum Controller hinauf, alle Aufrufer mit "throws" im Methodenkopf schmücken darf.

Jemand Vorschläge?

mfg

 

[MeinerEiner_80]

Moin!
*Just my 2 Cents*
Laut deinem Methodennamen möchtest du überprüfen, ob einen Datei gültig ist. Ich würde da eher einen Wahrheitswert erwarten.
Eine Exception würde ich eher da erwarten, wenn du auf eine Datei zugreifen möchtest, von der du erwartest, das sie gültig ist, sie es dann aber doch nicht ist.
Ob du dann diese Exception gleich in dieser Methode in ein try/catch bettest oder erst über ein "throws" weiterleitest, hängt stark von der Anwendung ab. Prinzipiell kann man eigentlich nur sagen, das die Exception da verarbeitet werden sollte, wo dann geeignete Gegenmaßnahmen ergriffen werden können... Du musst also nicht alle aufrufenden Methoden bis hoch zum Controller mit einem throw schmücken. Eventuell macht eine Bearbeitung auch schon früher Sinn.. Aber das kommt dann halt wirklich auf die Anwendung an...


*grüssle*
MeinerEiner

 

[y0dA]

Jo nur in diesem Fall, müsste die Exception auch bis zum Controller, da ich dem Benutzer mehr oder weniger mitteilen sollte, dass das File ungültig (was auch immer) ist.

 

[zerix]

Hallo,

also ich würde da auch einen boolean-Wert erwarten. Grob gesagt, wird ja die Frage gestellt, ob die Datei gültig ist. Als Antwort gibts dann ja oder nein.

Meiner Meinung nach solltest du da einen boolean-wert zurück geben. Wenn du später eine Exception möchtest, kannst du ja die Methode aufrufen und wenn false zurück gegeben wird, kannst du ja eine Exception werfen.

MFG

zEriX

 

[y0dA]

Also im Grunde genommen ist die Sachlage wie folgt:
Ich habe ein Interface, in welchem folgender Methodenkopf steht:

public void storeTracksFromFile(final String fPath, final String fName, final Class className,
			final Receiver receiver);

Die Implementierung hierzu sieht dann wie folgt aus:

	public void storeTracksFromFile(final String fPath, final String fName, final Class className,
			final Receiver receiver) {
		try {
			FileSystemProcessor fsp = new FileSystemProcessor(fPath, fName);
			if (fsp.isFileValid()) {
				fsp.setItrackParser((ITrackParser) className.newInstance());
				this.storeTracksFromFile(fsp, fPath, fName, receiver);
			} else {
				//FIXME was hier machen
			}
		} catch (IllegalAccessException e) {
//			FIXME was hier machen
		} catch (InstantiationException e) {
//			FIXME was hier machen
		}
	}

Hier stellt sich nun die Frage ob ich die Interface Methode ändern sollte und jener einen boolean Wert statt void als Rückgabewert verpassen sollte?

Ich weiß halt nicht wirklich wie ich hier vorgehen soll, also Exception (checked oder unchecked) werfen oder nur mit boolean arbeiten --> Ziel sollte auf jeden Fall sein, dass der Aufrufer der Interface Methode weiß, ob das speichern geklappt hat oder nicht

 

[MeinerEiner_80]

Ziel sollte auf jeden Fall sein, dass der Aufrufer der Interface Methode weiß, ob das speichern geklappt hat oder nicht

Wenn der Nutzer nur wissen soll, ob es geklappt hat oder nicht, dann reicht der Boolean Wert aus. Wenn du ihm darüberhinaus auch noch mitteilen möchtest, warum es nicht geklappt hat, dann würde ich eine Exception werfen...

*grüssle*
MeinerEiner

 

[y0dA]

Wenn der Nutzer nur wissen soll, ob es geklappt hat oder nicht, dann reicht der Boolean Wert aus. Wenn du ihm darüberhinaus auch noch mitteilen möchtest, warum es nicht geklappt hat, dann würde ich eine Exception werfen...

*grüssle*
MeinerEiner

Klingt einleuchtend!
Nur wo soll ich dann die Exception werfen:
+) In der Methode isFileValid()
+) oder in der Methode storeTracksFromFile(..)?

Darüberhinaus stellt sich mir die Frage ob ich dann eine IOException werfen soll, wenn ja muss ich jene ja im Interface deklarieren oder eine RuntimeException?

 

[Oliver Gierke]

Ich fänd eine RuntimeException sinnvoller. Und zwar würde ich die aus dem Konstruktor des FileSystemProcessor werfen (Überprüfung dort machen und checked Exceptions in RE (vielleicht. IllegalArgumentException?) wrappen bzw. vielleicht eine eigene RE implementieren). Du kannst an den Stellen (nach isValid usw.) gar nicht wirklich sinnvoll auf die Exception reagieren ausser sie weiterzureichen. Die RE lässt sich ausserdem mithilfe von AOP recht einfach an den Stellen abgreifen, an denen sie fachlich sinnvoll sind. Das können bei unterschieldlichen Clients sehr unterschiedliche stellen sein.

Die Vorteile:

* dein Code wird wesentlich einfacher. storeTracksFromFile beinhaltet nur noch die eigentliche Logik
* FileSystemProcessor wird eine "Komponente" die beim initialisieren gleich validen Zustand aufweist (in der jetzigen Variante kannst du das Ding invalid instantiieren und merkst das erst beim Zugriff auf Methoden)

"Nachteil":
* an der Schnittstelle ist nicht mehr ersichtlich, dass die Exceptions fliegen können (man kann REs allerdings auch mit throws deklarieren, macht nur kaum jemand)

Workaround hierfür ist die Dokumentation der REs am Konstruktor. Unittests für FileSystemProcessor sollten sicherstellen, dass die RE unter den dokumentierten Bedingungen geworfen werden.

Gruß
Ollie

 

[y0dA]

Jo die Idee mit dem Konstruktor finde ich gut

Hast dus dir so vorgestellt:

public FileSystemProcessor(final String jarPath, final String jarName, final String className) {
		try {
			if (this.isPathValid(jarPath)) {
				this.setJarPath(jarPath);
			} else {
				throw new IOException("path: " + jarPath + "is not a directory");
			}
			if (this.isFileValid(jarPath, jarName)) {
				this.setJar(jarName);
			} else {
				throw new JarException("jar: " + jarName + "cannot found or read jar");
			}
			this.setClassName(className);	
		} catch	(IOException e) {
			throw new RuntimeException(e);
		}
	}

mfg
Anmerkung: Nun beginnste schon wieder mit Spring (Sprichwort AoP)

 

[Oliver Gierke]

Grundsätzlich ja, allerdings würde ich die if else throw Konstrukte wegwerfen und einfach als erstes die validierenden Operationen ausführen. Nicht unbedingt in einer Methode die so klingt als würde sie einen Boolean zurückgeben. Das sieht zu sehr danach aus, als wäre sie für den Client geschrieben. Vielleicht so:

public FileSystemProcessor(final String jarPath, final String jarName, final String className) throws IllegalArgumentException {

  try {

    validateParameters(jarPath, jarName); // Hier validieren, schöneren Methodennamen suchen
    setJarPath(jarPath);
    setJar(jarName);
  } catch (IOException e) {
    throw new IllegalArgumentException(e); // Evtl mit ner Fehlermeldung aufwerten
  }
}

private void validateParameters(String jarPath, String jarName) throws IOException {
  // Validierung implmentieren
}

Wenn du jetzt den Fehler irgendwo bearbeiten willst, ist es evtl. sinnvol IllegalArgumentException durch eine selbstgeschriebene InvalidJarSpecificationException (ebenfalls RE) zu ersetzen. Dann haben es die Pointcuts leichter .

@Spring/AOP: Benutz AspectJ und du hast AOP ohne Spring. Dass du auch immer gleich böses vermutest

REINHAUN!