3

I have this piece of code that would copy files from IFS to a local drive. And I would like to ask some suggestions on how to make it better.

public void CopyFile(AS400 system, String source, String destination){
    File destFile = new File(destination);
    IFSFile sourceFile = new IFSFile(system, source);
    if (!destFile.exists()){
        try {
            destFile.createNewFile();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
            IFSFileInputStream in = null;
    OutputStream out = null;
    try {
         in = new IFSFileInputStream(sourceFile);
         out = new FileOutputStream(destFile);

            // Transfer bytes from in to out
            byte[] buf = new byte[1024];
            int len;
            while ((len = in.read(buf)) > 0) {
                out.write(buf, 0, len);
            }
        } catch (AS400SecurityException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            try {
                if(in != null) {
                    in.close();
                }
                if(out != null) {
                    out.close();
                }
            } catch (IOException e) {
                e.printStackTrace();
            }
        } // end try catch finally

} // end method

Where

  • source = full IFS path + filename and
  • destination = full local path + filename

I would like to ask some things regarding the following:

  • a. Performance considerations

    1. would this have a big impact in terms for CPU usage for the host AS400 system?
    2. would this have a big impact on the JVM to be used (in terms of memory usage)
    3. would including this to a web app affect app server performance (would it be a heavy task or not)?
    4. would using this to copy multiple files (running it redundantly) be a big burden to all resources involved?
  • b. Code Quality

    1. Did my implementation of IFSFileInputStream suffice, or would a simple FileInputStream object do the job nicely?

AFAIK, I just needed the AS400 object to make sure the source file referenced is a file from IFS.

I am a noob at AS400 and IFS an would like to ask an honest opinion from experienced ones.

Gilles 'SO- stop being evil'
  • 104,111
  • 38
  • 209
  • 254
aEtherv0id
  • 31
  • 1
  • 2

2 Answers2

1

All in all it looks fine (without trying). It should not have a noticeable impact.

  • in.read() may return 0. Test for -1 instead.
  • Instead of manually buffering, just wrap in and out with their respective BufferedInputStream/BufferedOutputstream and read one character at a time and test it for -1.
  • try-catch is hard to get pretty. This will do, but you will later get more experience and learn how to do it somewhat better.
  • Do NOT swallow exceptions and print them. The code calling you will have no idea whether it went well or not.
  • When done with an AS400 object, use as400.disconnectAllServices().
Thorbjørn Ravn Andersen
  • 73,784
  • 33
  • 194
  • 347
  • Thank you so much for your quick answer Thorbjørn. I also thought of as400.disconnectAllServices() part, and that would be a part of my cleanup method (destroy) for my class. And yes, I hope that I would soon find a way to better handle the try-catch block nicely. I'm also planning on logging the exceptions using log4j instead of doing printStackTrace. But how do I handle the exception and inform the calling code nicely? Thanks again. – aEtherv0id Jun 26 '12 at 10:57