You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2011/11/23 22:24:09 UTC

svn commit: r1205606 - /jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java

Author: pmouawad
Date: Wed Nov 23 21:24:09 2011
New Revision: 1205606

URL: http://svn.apache.org/viewvc?rev=1205606&view=rev
Log:
Simplify / Clarify code by using Closeable interface

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java

Modified: jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java?rev=1205606&r1=1205605&r2=1205606&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java Wed Nov 23 21:24:09 2011
@@ -23,6 +23,7 @@ package org.apache.jmeter.services;
 
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
@@ -39,6 +40,7 @@ import org.apache.commons.collections.Ar
 import org.apache.jmeter.gui.JMeterFileFilter;
 import org.apache.jmeter.util.JMeterUtils;
 import org.apache.jorphan.logging.LoggingManager;
+import org.apache.jorphan.util.JOrphanUtils;
 import org.apache.log.Logger;
 
 /**
@@ -393,13 +395,7 @@ public class FileServer {
     private void closeFile(String name, FileEntry fileEntry) throws IOException {
         if (fileEntry != null && fileEntry.inputOutputObject != null) {
             log.info("Close: "+name);
-            if (fileEntry.inputOutputObject instanceof Reader) {
-                ((Reader) fileEntry.inputOutputObject).close();
-            } else if (fileEntry.inputOutputObject instanceof Writer) {
-                ((Writer) fileEntry.inputOutputObject).close();
-            } else {
-                log.error("Unknown inputOutputObject type : " + fileEntry.inputOutputObject.getClass());
-            }
+            JOrphanUtils.closeQuietly(fileEntry.inputOutputObject);
             fileEntry.inputOutputObject = null;
         }
     }
@@ -437,9 +433,9 @@ public class FileServer {
     private static class FileEntry{
         private String headerLine;
         private final File file;
-        private Object inputOutputObject; // Reader/Writer
+        private Closeable inputOutputObject; 
         private final String charSetEncoding;
-        FileEntry(File f, Object o, String e){
+        FileEntry(File f, Closeable o, String e){
             file=f;
             inputOutputObject=o;
             charSetEncoding=e;



Re: svn commit: r1205606 - /jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java

Posted by sebb <se...@gmail.com>.
On 23 November 2011 22:43, sebb <se...@gmail.com> wrote:
> On 23 November 2011 21:24,  <pm...@apache.org> wrote:
>> Author: pmouawad
>> Date: Wed Nov 23 21:24:09 2011
>> New Revision: 1205606
>>
>> URL: http://svn.apache.org/viewvc?rev=1205606&view=rev
>> Log:
>> Simplify / Clarify code by using Closeable interface
>>
>> Modified:
>>    jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java
>>
>> Modified: jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java
>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java?rev=1205606&r1=1205605&r2=1205606&view=diff
>> ==============================================================================
>> --- jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java (original)
>> +++ jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java Wed Nov 23 21:24:09 2011
>> @@ -23,6 +23,7 @@ package org.apache.jmeter.services;
>>
>>  import java.io.BufferedReader;
>>  import java.io.BufferedWriter;
>> +import java.io.Closeable;
>>  import java.io.File;
>>  import java.io.FileInputStream;
>>  import java.io.FileOutputStream;
>> @@ -39,6 +40,7 @@ import org.apache.commons.collections.Ar
>>  import org.apache.jmeter.gui.JMeterFileFilter;
>>  import org.apache.jmeter.util.JMeterUtils;
>>  import org.apache.jorphan.logging.LoggingManager;
>> +import org.apache.jorphan.util.JOrphanUtils;
>>  import org.apache.log.Logger;
>>
>>  /**
>> @@ -393,13 +395,7 @@ public class FileServer {
>>     private void closeFile(String name, FileEntry fileEntry) throws IOException {
>>         if (fileEntry != null && fileEntry.inputOutputObject != null) {
>>             log.info("Close: "+name);
>> -            if (fileEntry.inputOutputObject instanceof Reader) {
>> -                ((Reader) fileEntry.inputOutputObject).close();
>> -            } else if (fileEntry.inputOutputObject instanceof Writer) {
>> -                ((Writer) fileEntry.inputOutputObject).close();
>> -            } else {
>> -                log.error("Unknown inputOutputObject type : " + fileEntry.inputOutputObject.getClass());
>> -            }
>> +            JOrphanUtils.closeQuietly(fileEntry.inputOutputObject);
>
> This changes the behaviour, in that the code no longer throws IOException.
>
> I'm not convinced that is the correct thing to do, especially for a Writer.
>
> Could still simplify the code by using
>
>      fileEntry.inputOutputObject.close()
>
> instead of closeQuietly.

I've implemented this for now.

Note that swallowing the exception meant that the method no longer
threw IOException.
I tried fixing the warning by removing the throws clause.
Had to do thw same with the calling method => this causes compile
errors in the main JMeter code.

So we need to be careful when changing exception handling.

> We can then catch the IOE later if necessary and take appropriate
> action, e.g. log it.
>
>>             fileEntry.inputOutputObject = null;
>>         }
>>     }
>> @@ -437,9 +433,9 @@ public class FileServer {
>>     private static class FileEntry{
>>         private String headerLine;
>>         private final File file;
>> -        private Object inputOutputObject; // Reader/Writer
>> +        private Closeable inputOutputObject;
>>         private final String charSetEncoding;
>> -        FileEntry(File f, Object o, String e){
>> +        FileEntry(File f, Closeable o, String e){
>>             file=f;
>>             inputOutputObject=o;
>>             charSetEncoding=e;
>>
>>
>>
>

Re: svn commit: r1205606 - /jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java

Posted by sebb <se...@gmail.com>.
On 23 November 2011 21:24,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Wed Nov 23 21:24:09 2011
> New Revision: 1205606
>
> URL: http://svn.apache.org/viewvc?rev=1205606&view=rev
> Log:
> Simplify / Clarify code by using Closeable interface
>
> Modified:
>    jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java
>
> Modified: jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java?rev=1205606&r1=1205605&r2=1205606&view=diff
> ==============================================================================
> --- jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java (original)
> +++ jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java Wed Nov 23 21:24:09 2011
> @@ -23,6 +23,7 @@ package org.apache.jmeter.services;
>
>  import java.io.BufferedReader;
>  import java.io.BufferedWriter;
> +import java.io.Closeable;
>  import java.io.File;
>  import java.io.FileInputStream;
>  import java.io.FileOutputStream;
> @@ -39,6 +40,7 @@ import org.apache.commons.collections.Ar
>  import org.apache.jmeter.gui.JMeterFileFilter;
>  import org.apache.jmeter.util.JMeterUtils;
>  import org.apache.jorphan.logging.LoggingManager;
> +import org.apache.jorphan.util.JOrphanUtils;
>  import org.apache.log.Logger;
>
>  /**
> @@ -393,13 +395,7 @@ public class FileServer {
>     private void closeFile(String name, FileEntry fileEntry) throws IOException {
>         if (fileEntry != null && fileEntry.inputOutputObject != null) {
>             log.info("Close: "+name);
> -            if (fileEntry.inputOutputObject instanceof Reader) {
> -                ((Reader) fileEntry.inputOutputObject).close();
> -            } else if (fileEntry.inputOutputObject instanceof Writer) {
> -                ((Writer) fileEntry.inputOutputObject).close();
> -            } else {
> -                log.error("Unknown inputOutputObject type : " + fileEntry.inputOutputObject.getClass());
> -            }
> +            JOrphanUtils.closeQuietly(fileEntry.inputOutputObject);

This changes the behaviour, in that the code no longer throws IOException.

I'm not convinced that is the correct thing to do, especially for a Writer.

Could still simplify the code by using

      fileEntry.inputOutputObject.close()

instead of closeQuietly.

We can then catch the IOE later if necessary and take appropriate
action, e.g. log it.

>             fileEntry.inputOutputObject = null;
>         }
>     }
> @@ -437,9 +433,9 @@ public class FileServer {
>     private static class FileEntry{
>         private String headerLine;
>         private final File file;
> -        private Object inputOutputObject; // Reader/Writer
> +        private Closeable inputOutputObject;
>         private final String charSetEncoding;
> -        FileEntry(File f, Object o, String e){
> +        FileEntry(File f, Closeable o, String e){
>             file=f;
>             inputOutputObject=o;
>             charSetEncoding=e;
>
>
>