Datei kopiermethode verbesserungswürdig?

[SchönerDöner]

Hallo,

hab mal ne alte methode von mir rausgekramt heute. Sieht irgendwie verbesserungswürdig aus ^^ was meint Ihr:

public void copyFile(String source , String target) throws IOException
	{		   
		   File inputFile = new File(source);
		   File outputFile = new File(target);
		   File[] allFiles = inputFile.listFiles();  // get all files from given directory name 
		   
		   FileInputStream in = null;
		   FileOutputStream out = null;

		   FileChannel inc = null;
		   FileChannel outc = null;		   
		  
		   for(int i = 0 ; i < allFiles.length ; i++)
		   {			   
			   inputFile = new File(source + "/" + allFiles[i].getName()); 			 
			   outputFile = new File(target + "/" + allFiles[i].getName());
			   
			   in = new FileInputStream(inputFile);
			   out = new FileOutputStream(outputFile);
			   
			   inc = in.getChannel();
			   outc = out.getChannel();
			   
			   inc.transferTo(0,inc.size(),outc);
		   } 
		   inc.close();
		   outc.close();
		   in.close();
		   out.close();
	}

 

[FArt]

Auf den ersten Blick:

- Channel und Stream müssen nicht beide geschlossen werden

- Bei einem Fehler werden die Streams nicht geschlossen, somit muss das in einem finally-Block um die Aktion geschehen

- Verwende von File den Konstrktor File(File, String) oder File(String, String), dann musst du nicht mit den Separatoren rumpopeln

- FileChannel würde ich in der Schleife und final deklarieren

- Bis auf die Streams, die im finally-Block geschlossen gehören, kannst du alle Variablen final deklarieren

- listFiles liefert auch Verzeichnisse, das wird transferTo ganz schön durcheinander bringen

evtl. noch: Fehler in der Schleife verarbeiten, falls ein Fehler nicht verhindern soll, dass der Rest nicht kopiert wird.

Gutes Beispiel für suboptimalen Code, der aber schön anzusehen ist ;-)

 

[diggaa1984]

was genau bringt das wenn er den Channel in der Schleife als final deklariert, ist das schneller/besser als wenn er es vor der Schleife deklariert? Ich weiss was ne finale lokale Variable ausmacht, aber die Verbesserung entzieht sich meinem Wissen ^^

 

[Wildcard]

Bring eigentlich nichts (mehr).
Deine Streams musst du allerdings in der Schleife schließen, denn dort machst du sie auch auf.
So schließt du immer nur den Letzten.

 

[FArt]

was genau bringt das wenn er den Channel in der Schleife als final deklariert, ist das schneller/besser als wenn er es vor der Schleife deklariert? Ich weiss was ne finale lokale Variable ausmacht, aber die Verbesserung entzieht sich meinem Wissen ^^

1. Guter Code ist selbstdokumentierend
2. fianal in dem kleinen Scope sorgt dafür, dass die Objekte in der Regel lediglich in der young generation leben und schnell und effektiv abgeräumt werden können

Das ist kein Fehler, aber ich würde das bei einem Codereview als unschön anmerken.

 

[Wildcard]

2. fianal in dem kleinen Scope sorgt dafür, dass die Objekte in der Regel lediglich in der young generation leben und schnell und effektiv abgeräumt werden können

Ich behaupte jetzt einfach mal, dass der Compiler das mittlerweile selbstständig erledigt. Schließlich ist ihm bekannt, ob die Variable final sein kann, oder nicht.

 

[FArt]

2. fianal in dem kleinen Scope sorgt dafür, dass die Objekte in der Regel lediglich in der young generation leben und schnell und effektiv abgeräumt werden können

Ich behaupte jetzt einfach mal, dass der Compiler das mittlerweile selbstständig erledigt. Schließlich ist ihm bekannt, ob die Variable final sein kann, oder nicht.

Jein. Der Compiler erkennt das, aber erst nach sehr vielen Durchläufen. So lange ist der Hinweis für den Compiler sinnvoll....

Wie gesagt, hier geht es weniger um die Performance, mehr um Codehygiene und guten Stil... und für solche Hinweise wird sich ein Compiler zur Laufzeit sicherlich erkenntlich zeigen...

 

[Wildcard]

Ich rede nicht vom JIT, sondern vom statischen Compiler.