1

I am trying to write an FTP client program and I have input and output streams to transfer files between server and my computer.

When I was thinking about how to design the class, I couldn't decide on whether to open up a new InputStream every time I call the function and then immediately close it (as in the example below).

Or just do this in the constructor and close it while quitting the program. Does it matter? Does it make sense to do it this way, especially if the user has the option to decide to upload another file to the server immediately after doing it once? 

public class FTPClientP extends FTP{
    private InputStream is;

    public FTPC(String serverAdd, int connection_port){
        is = null;

    }
    public int connectToServer() throws  IOException{

    }

    public boolean uploadToServer(File file) throws IOException{

        boolean uploaded = false;

        is = new FileInputStream(file);

        String fileName = myFile.getName();

        uploaded = ftp.storeFile(fileName, is);

        is.close();

        return uploaded;
    }

}
Rahul Rahatal
  • 648
  • 5
  • 19
C. O.
  • 125
  • 9
  • 4
    You should only define a stream (or other resource) in the shortest scope necessary. Use try-with-resources to ensure it is closed after you're done. So defining it as a field, when you only need it for the lifetime of a method call is the wrong approach. – Mark Rotteveel Feb 02 '19 at 08:49
  • 2
    According to what I was taught: All actions with resources (streams, database connections, etc) should be as brief as possible so as to avoid dangling handlers (left open and not closed) and memory leaks. Get in, get what you need and get out in as short a time frame as possible. It's cleaner to open and close a connection for each of a multiple of request methods rather than open a connection at the start of the program, pass it to the methods that make requests and only close it at the end of the program. – Agi Hammerthief Feb 02 '19 at 08:50

1 Answers1

2

You should open and close InputStreams,OutputStreams, and other resources like that, as soon as possible (in the nearest scope possible). For example, if i want to send a file the steps are

  1. Open an OutputStream.
  2. Send bytes.
  3. Close OutputStream.

If you will not close such resources , you will be facing a memory leak.

You can use try-with resources so you will not accidentally forget to close your resource. You can use any resource you like with try-with resources, as long as it implements the AutoClosable interface. (InputStream and OutputStream indeed implement the AutoClosable interface).

an example of using try-with resources: 

try (InputStream fis = new FileInputStream(source);
        OutputStream fos = new FileOutputStream(target)){

        byte[] buf = new byte[8192];

        int i;
        while ((i = fis.read(buf)) != -1) {
            fos.write(buf, 0, i);
        }
    }
    catch (Exception e) {
        e.printStackTrace();
    }

Note: both the InputStream AND the OutputStream, are in the try-with resources statement, in the example above.

Daniel B.
  • 2,491
  • 2
  • 12
  • 23
  • 1
    I agree, but note that there might be situations in which you want to keep a connection open though. For example if you know that you soon will need to write to it again and performance is an issue. Remember that opening and closing is a rather heavy operation. – Zabuzard Feb 02 '19 at 09:37
  • 1
    @Zabuza If you are doing operations that are back-to-back, than sure, keep it open. Otherwise, it is best practice to close them down. – Daniel B. Feb 02 '19 at 09:39