2
\$\begingroup\$

(Also, see the next iteration.)

I have this tiny class for downloading files from internet:

package com.github.coderodde.utils.io;

import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.Objects;

/**
 * This class implements a downloadable remote file.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Mar 14, 2020) ~ initial Happy Pi Day -version.
 * @since 1.6 (Mar 14, 2020)
 */
public class RemoteFile {

    /**
     * The URL of the target remote file.
     */
    private String url;

    /**
     * Constructs a new {@code RemoteFile} object with given URL as a string.
     * 
     * @param url the URL of the target remote file.
     */
    public RemoteFile(String url) {
        this.url = Objects.requireNonNull(url, "The URL is null.");
    }

    /**
     * Downloads the remote file to local disk.
     * 
     * @param path the path of the target file on the local disk.
     * 
     * @throws MalformedURLException if there are problems with URL.
     * 
     * @throws IOException if I/O fails.
     */
    public void download(String path) throws MalformedURLException,
                                             IOException {
        InputStream inputStream = new URL(url).openStream();
        Files.copy(inputStream, 
                   Paths.get(path), 
                   StandardCopyOption.REPLACE_EXISTING);
    }
}

Critique request

I would like to hear any comments. In particular, I would like to know what other functionalities I could add to it.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Maybe add a callback / blocking call for when it is downloaded? \$\endgroup\$
    – tieskedh
    Mar 14, 2020 at 19:52
  • \$\begingroup\$ @tieskedh I suspect downoad blocks until done downloading. \$\endgroup\$
    – coderodde
    Mar 15, 2020 at 9:07
  • \$\begingroup\$ Depends on the use-case... If there is a possibility to do it with callbacks, then you can do it (maybe singlethreaded) while doing something else (advanced topic, but you could look into completeableFutures). At other moments, you need to wait for the result. Both have use cases, but you're the programmer 😉 \$\endgroup\$
    – tieskedh
    Mar 15, 2020 at 9:25

1 Answer 1

2
\$\begingroup\$

It would probably be good to directly in the RemoteFile constructor perform the conversion to URL to allow the code to fail fast.

The code should also close the InputStream retrieved from openStream() once the copying is done, e.g. using a try-with-resources statement.

In general (unless required by your use case) it might be even better to directly use URL as type for the url parameter of the constructor and Path as type for the download(...) argument. This way your are handing off validation / parsing responsiblity to the caller.

And it might be good to make sure the path argument of download(...) is non-null before opening the InputStream.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.