5
\$\begingroup\$

I have written a simple download accelerator in java which downloads the same file in multiple threads and then combines them all.

I would love some feedback on my design. How the same implementation can be better written. Idioms, conventions, anything that comes to your mind. I hope I have not included a lot of code here. I thought it would be hard to review my code without the basic classes.

If you want to view the project in a more organized manner, the project is public in GitHub here

Thank you in advance. I have included a basic introduction to each of the classes along with the code. Hope that helps to review my code.

How the program looks

Screenshot

DownloadManager.java

This class is the main starting point of the application. It starts up a GUI interface made in JavaFX, and creates a DownloadPool object. DownloadPool is the object that is responsible for managing the download of the files. This class is just the GUI interface for the actual program.

package downloadmanager;

import java.io.IOException;
import javafx.application.Application;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;
import javafx.scene.control.TextField;
import javafx.scene.layout.HBox;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

/**
 *
 * @author gnik
 */
public class DownloadManager extends Application {

    DownloadPool downloadPool = new DownloadPool().load();
    Stage window;
    TableView<DownloadThread> table;

    Label urlLabel = new Label("URL:");
    TextField urlInput = new TextField();

    Button newDownload = new Button("Download");
    Button pauseButton = new Button("Pause");
    Button resumeButton = new Button("Resume");
    Button stopButton = new Button("Stop");
    Button removeButton = new Button("Remove");

    /**
     * @param args the command line arguments
     * @throws java.io.IOException
     * @throws java.lang.InterruptedException
     */
    public static void main(String args[]) throws IOException, InterruptedException {
        launch(args);
    }

    @Override
    public void stop() {
        downloadPool.stopAll();
        downloadPool.joinThreads();
        downloadPool.save();
    }

    public void setTable() {
        TableColumn<DownloadThread, Integer> idColumn = new TableColumn<>("ID");
        idColumn.setMinWidth(50);
        idColumn.setCellValueFactory((TableColumn.CellDataFeatures<DownloadThread, Integer> download) -> download.getValue().getDownloadMetadata().getDownloadIDProperty());

        TableColumn<DownloadThread, String> urlColumn = new TableColumn<>("URL");
        urlColumn.setMinWidth(200);
        urlColumn.setCellValueFactory((TableColumn.CellDataFeatures<DownloadThread, String> download) -> download.getValue().getDownloadMetadata().getUrlProperty());

        TableColumn<DownloadThread, String> statusColumn = new TableColumn<>("Status");
        statusColumn.setMinWidth(200);
        statusColumn.setCellValueFactory((TableColumn.CellDataFeatures<DownloadThread, String> download) -> download.getValue().getDownloadMetadata().getStatusProperty());

        TableColumn<DownloadThread, String> filenameColumn = new TableColumn<>("Filename");
        filenameColumn.setMinWidth(150);
        filenameColumn.setCellValueFactory((TableColumn.CellDataFeatures<DownloadThread, String> download) -> download.getValue().getDownloadMetadata().getFilenameProperty());

        TableColumn<DownloadThread, String> sizeColumn = new TableColumn<>("Size");
        sizeColumn.setMinWidth(100);
        sizeColumn.setCellValueFactory((TableColumn.CellDataFeatures<DownloadThread, String> download) -> download.getValue().getDownloadMetadata().getSizeProperty());

        TableColumn<DownloadThread, String> acceleratedColumn = new TableColumn<>("Accelerated");
        acceleratedColumn.setMinWidth(50);
        acceleratedColumn.setCellValueFactory((TableColumn.CellDataFeatures<DownloadThread, String> download) -> download.getValue().getDownloadMetadata().getAcceleratedProperty());
        table = new TableView();

        table.setItems(downloadPool.getDownloadThreads());
        table.getColumns().addAll(idColumn, urlColumn, statusColumn, filenameColumn, sizeColumn, acceleratedColumn);

    }

    public void setButtons() {
        newDownload.setOnAction(new EventHandler<ActionEvent>() {
            @Override
            public void handle(ActionEvent eh) {
                downloadPool.newDownload(urlInput.getText());
                urlInput.clear();
            }
        });

        pauseButton.setOnAction(new EventHandler<ActionEvent>() {
            @Override
            public void handle(ActionEvent eh) {
                downloadPool.pauseDownload(table.getSelectionModel().getSelectedItem());
            }
        });

        resumeButton.setOnAction(new EventHandler<ActionEvent>() {
            @Override
            public void handle(ActionEvent eh) {
                downloadPool.resumeDownload(table.getSelectionModel().getSelectedItem());
            }
        });

        stopButton.setOnAction(new EventHandler<ActionEvent>() {
            @Override
            public void handle(ActionEvent eh) {
                downloadPool.stopDownload(table.getSelectionModel().getSelectedItem());
            }
        });

        removeButton.setOnAction(new EventHandler<ActionEvent>() {
            @Override
            public void handle(ActionEvent eh) {
                downloadPool.removeDownload(table.getSelectionModel().getSelectedItem());
            }
        });

    }

    @Override
    public void start(Stage stage) throws Exception {
        window = stage;
        window.setTitle("Download Manager");
        setTable();
        setButtons();
        urlInput.setMinWidth(400);

        HBox hBox = new HBox();
        hBox.getChildren().addAll(urlLabel, urlInput, newDownload);
        hBox.setSpacing(10);

        HBox buttonList = new HBox();
        buttonList.getChildren().addAll(pauseButton, stopButton, resumeButton, removeButton);
        buttonList.setSpacing(10);

        VBox vBox = new VBox();
        vBox.getChildren().addAll(hBox, buttonList, table);
        Scene scene = new Scene(vBox);
        scene.getStylesheets().add("/resources/modena_dark.css");
        window.setScene(scene);
        window.show();
    }

}

DownloadPool.java

This class holds a bunch of DownloadThread objects. A DownloadThread object basically contains a thread that the download is running on, queues to communicate with the thread (for pausing/resuming downloads for example), and Download and DownloadMetadata objects that represent the downloads. This class is responsible for managing the state of all the Download objects.

package downloadmanager;

import java.net.MalformedURLException;
import java.net.URL;
import java.util.List;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.logging.Level;
import java.util.logging.Logger;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;

/**
 *
 * @author gnik
 */
public class DownloadPool {

    private final ObservableList<DownloadThread> downloadThreads = FXCollections.observableArrayList();
    DownloadSaves downloadSaves=new DownloadSaves();
    
    public DownloadPool() {
        downloadSaves.load();
    }
    
    
    public void save(){
        downloadSaves.clear();
        for (DownloadThread downloadThread:downloadThreads){
            DownloadState download;
            download=new DownloadState(downloadThread.getDownloadMetadata(),downloadThread.download.getValue().getPartMetadatas());
            downloadSaves.addDownload(download);
        }
        downloadSaves.save();
    }
    
    public DownloadPool load() {
        if(downloadSaves.getDownloads()==null){return this;}
        for (DownloadState downloadState : downloadSaves.getDownloads()) {
            
                DownloadMetadata downloadMetadata=downloadState.downloadMetadata;
                List<DownloadPartMetadata> downloadPartMetadata=downloadState.downloadPartMetadata;
                ConcurrentLinkedQueue queueCommand = new ConcurrentLinkedQueue();
                ConcurrentLinkedQueue queueResponse = new ConcurrentLinkedQueue();
                Download download = new Download(downloadMetadata, queueCommand, queueResponse);
                download.loadDownlaodPartMetadatas(downloadPartMetadata);
                Thread thread = new Thread(download);
                DownloadThread downloadThread = new DownloadThread(downloadMetadata, download, thread, queueCommand, queueResponse);
                downloadThreads.add(downloadThread);
                thread.start();

            
        }
        return this;
    }

    
    public boolean isValidUrl(String url) {
        try {
            URL test=new URL(url);
            return true;
        } catch (MalformedURLException ex) {
            return false;
        }
    }
    
    public ObservableList<DownloadThread> getDownloadThreads() {
        return downloadThreads;
    }


    private void waitUntilCommand(DownloadThread downloadThread,DownloadAction.Response command){
        while (true) {
            if(!downloadThread.queueResponse.isEmpty()){
                 if(downloadThread.queueResponse.peek().equals(command)){
                     downloadThread.queueResponse.poll();
                     break;
                 }
            }
        }
    }
    public void stopDownload(DownloadThread downloadThread) {
        if (!downloadThread.thread.isAlive()) {
            return;
        }
        downloadThread.queueCommand.add(DownloadAction.Command.STOP);
        waitUntilCommand(downloadThread,DownloadAction.Response.STOPPED);
        joinThread(downloadThread);

    }

    
    public void pauseDownload(DownloadThread downloadThread) {
        if (!downloadThread.thread.isAlive()) {
            return;
        }
        downloadThread.queueCommand.add(DownloadAction.Command.PAUSE);
        waitUntilCommand(downloadThread,DownloadAction.Response.PAUSED);
    }
    public void resumeDownload(DownloadThread downloadThread) {
        if (!downloadThread.thread.isAlive()) {
            return;
        }
        downloadThread.queueCommand.add(DownloadAction.Command.RESUME);
        waitUntilCommand(downloadThread,DownloadAction.Response.RESUMED);
    }
    
    public void removeDownload(DownloadThread downloadThread){
        if(downloadThread.thread.isAlive()){
            stopDownload(downloadThread);
            try {
                downloadThread.thread.join();
            } catch (InterruptedException ex) {
                Logger.getLogger(DownloadPool.class.getName()).log(Level.SEVERE, null, ex);
            }
        }
        downloadThreads.remove(downloadThread);
    }

    public void pauseAll() {
        for (DownloadThread downloadThread : downloadThreads) {
            pauseDownload(downloadThread);
        }
    }

    public void resumeAll() {
        for (DownloadThread downloadThread : downloadThreads) {
            resumeDownload(downloadThread);
        }
    }

    public void stopAll() {
        for (DownloadThread downloadThread : downloadThreads) {
            stopDownload(downloadThread);
        }
    }
    public void joinThread(DownloadThread downloadThread){
        try {
                downloadThread.thread.join();
            } catch (InterruptedException ex) {
                Logger.getLogger(DownloadPool.class.getName()).log(Level.SEVERE, null, ex);
            }
    }
    public void joinThreads() {
        for (DownloadThread downloadThread : downloadThreads) {
            joinThread(downloadThread);
        }
    }
    public void newDownload(String url) {
        DownloadMetadata downloadMetadata;
        try {
            downloadMetadata = new DownloadMetadata(url, downloadThreads.size());
        } catch (MalformedURLException ex) {
            Logger.getLogger(DownloadManager.class.getName()).log(Level.SEVERE, null, ex);
            return;
        }
        ConcurrentLinkedQueue queueCommand = new ConcurrentLinkedQueue();
        ConcurrentLinkedQueue queueResponse = new ConcurrentLinkedQueue();
        Download download = new Download(downloadMetadata, queueCommand, queueResponse);
        Thread thread = new Thread(download);
        DownloadThread downloadThread = new DownloadThread(downloadMetadata, download, thread, queueCommand, queueResponse);
        downloadThreads.add(downloadThread);
        thread.start();
    }

}

Download.java

This is the class that holds the download object. All the information about the download is stored in DownloadMetadata objects, and this class is responsible for handling all the actions such as pause/stop. The object also holds several DownloadPart objects, which as the name imply are the parts of the download. The download is separated into several DownloadPart objects and each part is simultaneously downloaded in several threads for accelerating download.

package downloadmanager;

import java.net.HttpURLConnection;
import java.io.*;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.logging.Level;
import java.util.logging.Logger;
import javafx.beans.property.SimpleObjectProperty;
import javafx.collections.FXCollections;

/**
 *
 * @author gnik
 */
public class Download implements Runnable {

    private final SimpleObjectProperty<DownloadMetadata> metadata;
    private final List<DownloadPartThread> downloadPartThreads = FXCollections.observableArrayList();
    private final ConcurrentLinkedQueue queueCommand;
    private final ConcurrentLinkedQueue queueResponse;

    public Download(DownloadMetadata metadata, ConcurrentLinkedQueue queueCommand, ConcurrentLinkedQueue queueResponse) {
        this.metadata = new SimpleObjectProperty<>(metadata);
        this.queueCommand = queueCommand;
        this.queueResponse = queueResponse;
    }

    @Override
    public String toString() {
        return "DownloadID:" + metadata.getValue().getDownloadID();
    }

    public DownloadMetadata getDownloadMetadata() {
        return metadata.getValue();
    }

    public SimpleObjectProperty<DownloadMetadata> getDownloadMetadataProperty() {
        return metadata;
    }

    public List<DownloadPartMetadata> getPartMetadatas() {
        List<DownloadPartMetadata> metadatas = new ArrayList<>();
        for (DownloadPartThread dthread : downloadPartThreads) {
            metadatas.add(dthread.getDownloadPartMetadata());
        }
        return metadatas;
    }

    public void setHeaders() throws IOException {
        HttpURLConnection conn;
        conn = (HttpURLConnection) getDownloadMetadata().getUrl().openConnection();
        conn.setRequestMethod("HEAD");
        getDownloadMetadata().setSize(conn.getContentLengthLong());
        String ranges = conn.getHeaderField("Accept-Ranges");
        if (ranges != null && !ranges.equals("none")) {
            getDownloadMetadata().setAccelerated(true);
            setStatus(DownloadStatus.STARTING);
        }

    }

    public void loadDownlaodPartMetadatas(List<DownloadPartMetadata> downloadPartMetadatas) {
        for (DownloadPartMetadata downloadPartMetadata : downloadPartMetadatas) {
            ConcurrentLinkedQueue queueCom = new ConcurrentLinkedQueue();
            ConcurrentLinkedQueue queueRes = new ConcurrentLinkedQueue();
            downloadPartMetadata.setDownloadMetadata(getDownloadMetadata());
            DownloadPart downloadPart = new DownloadPart(downloadPartMetadata, queueCom, queueRes);
            downloadPartThreads.add(new DownloadPartThread(downloadPart, downloadPartMetadata, queueCom, queueRes));
        }
    }

    public void createDownloadPartThreads() {
        int partID = 0;
        for (Part part : divideDownload()) {
            DownloadPartMetadata part_metadata = new DownloadPartMetadata(getDownloadMetadata(), partID, part);
            ConcurrentLinkedQueue queueCom = new ConcurrentLinkedQueue();
            ConcurrentLinkedQueue queueRes = new ConcurrentLinkedQueue();
            DownloadPart downloadPart = new DownloadPart(part_metadata, queueCom, queueRes);
            downloadPartThreads.add(new DownloadPartThread(downloadPart, part_metadata, queueCom, queueRes));
            partID++;
        }

    }

    public void initialize() {
        //If download Part Threads is not empty and loaded from file then skip.
        if (downloadPartThreads.isEmpty()) {
            try {
                setHeaders();

            } catch (IOException ex) {
                Logger.getLogger(Download.class.getName()).log(Level.SEVERE, null, ex);
                setStatus(DownloadStatus.ERROR);
                return;
            }
            createDownloadPartThreads();

        }
    }

    private List<Part> divideDownload() {
        List<Part> parts = new ArrayList<>();
        long start = 0;
        double size = (double) getDownloadMetadata().getSize() / getDownloadMetadata().getParts();
        for (int cnt = 0; cnt < getDownloadMetadata().getParts(); cnt++) {
            Part part = new Part(start, (int) Math.round(size * (cnt + 1)));
            parts.add(part);
            start = (int) Math.round(size * (cnt + 1)) + 1;

        }
        return parts;
    }

    private void setStatus(DownloadStatus downloadStatus) {
        getDownloadMetadata().setStatus(downloadStatus);
    }

    public DownloadStatus getStatus() {
        return getDownloadMetadata().getStatus();
    }

    public boolean isDownloaded() {
        for (DownloadPartThread downloadThread : downloadPartThreads) {
            if (downloadThread.getDownloadPart().getStatus() != DownloadStatus.COMPLETED) {
                return false;
            }
        }
        return true;
    }

    public void joinThread(Thread thread) {
        if (thread != null && !thread.isAlive()) {
            try {
                thread.join();
            } catch (InterruptedException ex) {
                Logger.getLogger(Download.class.getName()).log(Level.SEVERE, null, ex);
            }
        }
    }

    public void joinThreads() {
        for (DownloadPartThread downloadThread : downloadPartThreads) {
            joinThread(downloadThread.thread);
        }
    }

    public void waitUntilResponse(DownloadPartThread dthread, DownloadAction.Response response) {
        while (true) {
            if (!dthread.queueResponse.isEmpty() && dthread.queueResponse.peek().equals(response)) {
                dthread.queueResponse.poll();
                break;
            }
        }

    }

    public void pause() {
        if (getStatus() != DownloadStatus.DOWNLOADING) {
            return;
        }
        for (DownloadPartThread dthread : downloadPartThreads) {
            if (dthread.thread==null || !dthread.thread.isAlive()) {
                return;
            }
            dthread.queueCommand.add(DownloadAction.Command.PAUSE);
            waitUntilResponse(dthread, DownloadAction.Response.PAUSED);
        }

        setStatus(DownloadStatus.PAUSED);

    }

    public void resume() {
        if (getStatus() != DownloadStatus.PAUSED) {
            return;
        }
        for (DownloadPartThread dthread : downloadPartThreads) {
            if (dthread.thread==null || !dthread.thread.isAlive()) {
                return;
            }
            dthread.queueCommand.add(DownloadAction.Command.RESUME);
            waitUntilResponse(dthread, DownloadAction.Response.RESUMED);
        }
        setStatus(DownloadStatus.DOWNLOADING);
    }

    public void stop() {
        if (getStatus() == DownloadStatus.STOPPED) {
            return;
        }
        for (DownloadPartThread dthread : downloadPartThreads) {
            if (dthread.thread==null || !dthread.thread.isAlive()) {
                return;
            }
            dthread.queueCommand.add(DownloadAction.Command.STOP);
            waitUntilResponse(dthread, DownloadAction.Response.STOPPED);
        }

        setStatus(DownloadStatus.STOPPED);
    }

    public void startDownloadPartThreads() {
        if (!getDownloadMetadata().getAccelerated()) {
            setStatus(DownloadStatus.ERROR);
            return;
        }
        setStatus(DownloadStatus.DOWNLOADING);
        for (DownloadPartThread downloadThread : downloadPartThreads) {
            Thread thread = new Thread(downloadThread.getDownloadPart());
            thread.setName(this.toString() + " " + downloadThread.downloadPart.toString());
            downloadThread.thread = thread;
            thread.start();
        }
    }

    public void deleteDownloadPartFiles() throws IOException {
        for (DownloadPartThread downloadThread : downloadPartThreads) {
            DownloadPart downloadPart = downloadThread.getDownloadPart();
            Files.deleteIfExists(Paths.get(downloadPart.getFilename()));
        }
    }

    public void copyToStream(BufferedOutputStream outFile, BufferedInputStream inFile) throws IOException {
        int byt;
        while ((byt = inFile.read()) != -1 && outFile != null) {
            outFile.write(byt);
        }
    }

    public void joinDownloadParts() {
        if (!isDownloaded()) {
            return;
        }
        setStatus(DownloadStatus.JOINING);

        try(BufferedOutputStream outFile = new BufferedOutputStream(new FileOutputStream(getDownloadMetadata().getFilename()))) {
            for (DownloadPartThread downloadThread : downloadPartThreads) {
                DownloadPart downloadPart = downloadThread.getDownloadPart();
                try(BufferedInputStream inFile = new BufferedInputStream(new FileInputStream(downloadPart.getFilename()))){
                    copyToStream(outFile, inFile);
                }
            }
            setStatus(DownloadStatus.COMPLETED);
            deleteDownloadPartFiles();
        } catch (FileNotFoundException ex) {
            Logger.getLogger(Download.class.getName()).log(Level.SEVERE, null, ex);
        } catch (IOException ex) {
            Logger.getLogger(Download.class.getName()).log(Level.SEVERE, null, ex);
        }

    }
    public void downloadLoop(){
        while (!isDownloaded()) {
            try {
                Thread.sleep(100);
            } catch (InterruptedException ex) {
                setStatus(DownloadStatus.ERROR);
                Logger.getLogger(Download.class.getName()).log(Level.SEVERE, null, ex);
            }
            if (!this.queueCommand.isEmpty()) {
                DownloadAction.Command command = (DownloadAction.Command) this.queueCommand.poll();
                switch (command) {
                    case PAUSE:
                        this.pause();
                        this.queueResponse.add(DownloadAction.Response.PAUSED);
                        break;
                    case STOP:
                        this.stop();
                        this.joinThreads();
                        this.queueResponse.add(DownloadAction.Response.STOPPED);
                        return;
                    case RESUME:
                        this.resume();
                        this.queueResponse.add(DownloadAction.Response.RESUMED);
                        break;
                    default:
                        break;
                }
            }
        }
    }
    @Override
    public void run() {
        if (getDownloadMetadata().getStatus() == DownloadStatus.COMPLETED) {
            return;
        }
        this.initialize();
        this.startDownloadPartThreads();
        this.downloadLoop();
        this.joinThreads();
        this.joinDownloadParts();
    }

}

DownloadThread.java

This is the class which holds a few objects. Namely, a DownloadMetadata object, the Download object itself, the Thread object where the download will run, and the Queues which are used to communicate with the DownloadPool object. In essence, the DownloadPool object holds various DownloadThread objects to control all downloads.

package downloadmanager;


import java.util.concurrent.ConcurrentLinkedQueue;
import javafx.beans.property.SimpleObjectProperty;

/**
 *
 * @author gnik
 */
public class DownloadThread {

    public SimpleObjectProperty<DownloadMetadata> downloadMetadata;
    public SimpleObjectProperty<Download> download;
    public Thread thread;
    public ConcurrentLinkedQueue queueCommand;
    public ConcurrentLinkedQueue queueResponse;

    public DownloadThread(DownloadMetadata downloadMetadata, Download download, Thread thread, ConcurrentLinkedQueue queueCommand, ConcurrentLinkedQueue queueResponse) {
        this.downloadMetadata = new SimpleObjectProperty<>(downloadMetadata);
        this.download = new SimpleObjectProperty<>(download);
        this.thread = thread;
        this.queueCommand = queueCommand;
        this.queueResponse = queueResponse;
    }
    
    
    public Download getDownload(){
        return download.getValue();
    }
    public DownloadMetadata getDownloadMetadata() {
        return downloadMetadata.getValue();
    }
}

DownloadMetadata.java

This class holds all the information regarding the download such as the size, URL, number of parts in which the download is divided etc. This object is later serialized into the disk so that the downloads are persistent even though the program is closed.

package downloadmanager;

import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Paths;
import javafx.beans.property.SimpleObjectProperty;

/**
 *
 * @author gnik
 */
public class DownloadMetadata{
    private final SimpleObjectProperty<URL> url;
    private final SimpleObjectProperty<Integer> downloadID;
    private final SimpleObjectProperty<String> filename;
    private static final int parts=8;
    private final SimpleObjectProperty<Long> size=new SimpleObjectProperty<>();
    private static final int timeout=10000;
    private final SimpleObjectProperty<Boolean> accelerated=new SimpleObjectProperty<>(false);
    private final SimpleObjectProperty<DownloadStatus> status=new SimpleObjectProperty<>(DownloadStatus.NEW);
    
    public DownloadMetadata(String url,int ID) throws MalformedURLException{
        this.url=new SimpleObjectProperty<>(new URL(url));
        this.downloadID=new SimpleObjectProperty(ID);
        String file=String.valueOf(ID)+"_"+Paths.get(this.url.getValue().getPath()).getFileName().toString();
        this.filename=new SimpleObjectProperty<>(file);
    }

    public URL getUrl() {
        return url.getValue();
    }
    
    public SimpleObjectProperty getUrlProperty() {
        return url;
    }

    public Integer getDownloadID() {
        return downloadID.getValue();
    }
    
    
    public SimpleObjectProperty<Integer> getDownloadIDProperty() {
        return downloadID;
    }
    
    public String getFilename() {
        return filename.getValue();
    }
    public SimpleObjectProperty getFilenameProperty() {
        return filename;
    }

    public long getSize() {
        return size.getValue();
    }
    
    public SimpleObjectProperty getSizeProperty() {
        return size;
    }
    
    public void setSize(long s){
        size.setValue(s);
    }
    
    public DownloadStatus getStatus() {
        return status.getValue();
    }

    public SimpleObjectProperty getStatusProperty() {
        return status;
    }

    public void setStatus(DownloadStatus status) {
       this.status.setValue(status);
    }
    
    public boolean getAccelerated(){
        return accelerated.getValue();
    }
    
    public SimpleObjectProperty getAcceleratedProperty(){
        return accelerated;
    }
    
    public void setAccelerated(boolean a){
        accelerated.setValue(a);
    }
    public int getTimeout(){
        return timeout;
    }
    public int getParts(){
        return parts;
    }
}

DownloadPart.java

Okay, so this class is responsible for the download of one part of the download. As mentioned earlier, one single file download is divided into multiple parts. This class contains the methods to control the download of a specific part.

package downloadmanager;

import java.io.IOException;
import java.net.URLConnection;
import java.io.*;
import java.net.SocketTimeoutException;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.logging.Level;
import java.util.logging.Logger;
import javafx.beans.property.SimpleObjectProperty;

/**
 *
 * @author gnik
 */
public class DownloadPart implements Runnable {

    private final SimpleObjectProperty<DownloadPartMetadata> metadata;
    private final ConcurrentLinkedQueue queueCommand;
    private final ConcurrentLinkedQueue queueResponse;

    public DownloadPart(DownloadPartMetadata metadata, ConcurrentLinkedQueue queueCommand, ConcurrentLinkedQueue queueResponse) {
        this.queueCommand = queueCommand;
        this.queueResponse = queueResponse;
        this.metadata = new SimpleObjectProperty<>(metadata);

    }

    public DownloadPartMetadata getMetadata() {
        return metadata.getValue();
    }

    @Override
    public String toString() {
        return "DownloadPartID:" + getMetadata().partID;
    }

    public DownloadStatus getStatus() {
        return getMetadata().getStatus();
    }

    public void pause() {
        if (getMetadata().getStatus() == DownloadStatus.DOWNLOADING) {
            getMetadata().setStatus(DownloadStatus.PAUSED);
        }
    }

    public void resume() {
        if (getMetadata().getStatus() == DownloadStatus.PAUSED) {
            getMetadata().setStatus(DownloadStatus.DOWNLOADING);
        }
    }

    public void stop() {
        if (getMetadata().getStatus() == DownloadStatus.PAUSED || getMetadata().getStatus() == DownloadStatus.PAUSED) {
            getMetadata().setStatus(DownloadStatus.STOPPED);
        }
    }

    public String getFilename() {
        return getMetadata().getFilename();
    }

    public boolean isComplete() {
        return ((getMetadata().getCompletedBytes() + getMetadata().getPart().getStartByte()) == getMetadata().getPart().getEndByte());
    }

    private BufferedInputStream getConnectionStream() throws IOException {
        //Setting up the connection.
        URLConnection connection = getMetadata().downloadMetadata.getUrl().openConnection();
        connection.setRequestProperty("Range", "bytes=" + String.valueOf(getMetadata().getPart().getStartByte() + getMetadata().getCompletedBytes()) + "-" + String.valueOf(getMetadata().getPart().getEndByte()));
        connection.setConnectTimeout(5000);
        connection.setReadTimeout(getMetadata().downloadMetadata.getTimeout());
        connection.connect();

        BufferedInputStream inputStream = new BufferedInputStream(connection.getInputStream());
        return inputStream;
    }

    private boolean copyToStream(BufferedInputStream inputStream, BufferedOutputStream fileStream) throws IOException {
        int byt;
        long completedBytes = getMetadata().getCompletedBytes();

        while ((byt = inputStream.read()) != -1) {
            fileStream.write(byt);
            completedBytes++;
            getMetadata().setCompletedBytes(completedBytes);

            if (!queueCommand.isEmpty()) {
                if (queueCommand.peek().equals(DownloadAction.Command.PAUSE)) {
                    pause();
                    queueCommand.poll();
                    queueResponse.add(DownloadAction.Response.PAUSED);
                    return false;
                } else if (queueCommand.peek().equals(DownloadAction.Command.STOP)) {
                    stop();
                    //I am not adding a poll here because it will stop execution in run thread as well.
                    queueResponse.add(DownloadAction.Response.STOPPED);
                    return false;
                }
            }
        }
        return true;

    }

    public void download() throws IOException, SocketTimeoutException {
        getMetadata().setStatus(DownloadStatus.DOWNLOADING);
        boolean append = (getMetadata().getCompletedBytes() != 0);

        BufferedInputStream inputStream = getConnectionStream();
        BufferedOutputStream fileStream = new BufferedOutputStream(new FileOutputStream(getMetadata().filename, append));
        try {
            if (copyToStream(inputStream, fileStream)) {
                getMetadata().setStatus(DownloadStatus.COMPLETED);
            }
        } finally {
            inputStream.close();
            fileStream.close();
        }

    }

    public void safeDownload() {
        try {
            download();
        } catch (IOException ex) {
            getMetadata().setStatus(DownloadStatus.ERROR);
            getMetadata().incrementRetries();
            Logger.getLogger(DownloadPart.class.getName()).log(Level.SEVERE, null, ex);
        }
    }

    @Override
    public void run() {
        if (DownloadStatus.COMPLETED == getMetadata().getStatus()) {
            return;
        }
        safeDownload();
        //Infinite loop until the downloadstatus is completed 
        while (getMetadata().getStatus() != DownloadStatus.COMPLETED) {
            //Retry if there is any errors.
            if (getMetadata().getStatus() == DownloadStatus.ERROR) {
                safeDownload();
            }
            try {
                Thread.sleep(200);
            } catch (InterruptedException ex) {
                Logger.getLogger(DownloadPart.class.getName()).log(Level.SEVERE, null, ex);
            }

            if (!queueCommand.isEmpty()) {
                DownloadAction.Command command = (DownloadAction.Command) queueCommand.poll();
                switch (command) {
                    case STOP:
                        stop();
                        queueResponse.add(DownloadAction.Response.STOPPED);
                        return;
                    case RESUME:
                        resume();
                        queueResponse.add(DownloadAction.Response.RESUMED);
                        safeDownload();
                        break;
                    default:
                        break;
                }

            }
        }
    }
}

DownloadPartThread.java

This class is responsible for the storage of DownloadPart object and the thread it is running in. It contains a DownloadPart object, A download Metadata object and queue objects to communicate with the download object. A Download object contains several DownloadPartThread objects which enables the Download object to fully control the actions of each of the DownloadPart object and it's thread.

package downloadmanager;

import java.util.concurrent.ConcurrentLinkedQueue;
import javafx.beans.property.SimpleObjectProperty;

/**
 *
 * @author gnik
 */
public class DownloadPartThread {

    public Thread thread;
    public SimpleObjectProperty<DownloadPart> downloadPart;
    public ConcurrentLinkedQueue queueCommand;
    public ConcurrentLinkedQueue queueResponse;
    public SimpleObjectProperty<DownloadPartMetadata> downloadPartMetadata;

    public DownloadPartThread(DownloadPart downloadPart, DownloadPartMetadata downloadPartMetadata, Thread thread, ConcurrentLinkedQueue queueCommand, ConcurrentLinkedQueue queueResponse) {
        this.thread = thread;
        this.downloadPart = new SimpleObjectProperty<>(downloadPart);
        this.downloadPartMetadata = new SimpleObjectProperty<>(downloadPartMetadata);
        this.queueCommand = queueCommand;
        this.queueResponse = queueResponse;
    }

    public DownloadPartThread(DownloadPart downloadPart, DownloadPartMetadata downloadPartMetadata, ConcurrentLinkedQueue queueCommand, ConcurrentLinkedQueue queueResponse) {
        this.downloadPart = new SimpleObjectProperty<>(downloadPart);
        this.downloadPartMetadata = new SimpleObjectProperty<>(downloadPartMetadata);
        this.queueCommand = queueCommand;
        this.queueResponse = queueResponse;
        
    }
    
    public DownloadPart getDownloadPart(){
        return downloadPart.getValue();
    }
    
    public void setDownloadPart(DownloadPart t){
        downloadPart.setValue(t);
    }
    public DownloadPartMetadata getDownloadPartMetadata(){
        return downloadPartMetadata.getValue();
    }
}

DownloadPartMetadata.java

Similar to the DownloadMetadata object, this object holds the information regarding each of the part of the download. Such as size of each part, start and end byte, and the number of completed bytes. This object is also later serialized to the disk so that download can be paused and resumed later on.

package downloadmanager;

import com.thoughtworks.xstream.annotations.XStreamOmitField;
import javafx.beans.property.SimpleObjectProperty;

/**
 *
 * @author gnik
 */
public class DownloadPartMetadata{
   public SimpleObjectProperty<Integer> partID;
   public SimpleObjectProperty<DownloadStatus> status=new SimpleObjectProperty<>(DownloadStatus.STARTING);
   public String filename;
   
   //This field will be included multiple time if it is included
   @XStreamOmitField
   public DownloadMetadata downloadMetadata;
   
   public SimpleObjectProperty<Part> part;
   public SimpleObjectProperty<Long> completedBytes=new SimpleObjectProperty<>(0L);
   public SimpleObjectProperty<Integer> retries=new SimpleObjectProperty<>(0);


    public DownloadPartMetadata(DownloadMetadata downloadMetadata,int partID,Part part){
       this.downloadMetadata=downloadMetadata; 
       this.partID=new SimpleObjectProperty<>(partID);
       this.part=new SimpleObjectProperty<>(part);
       this.filename=downloadMetadata.getFilename()+".part"+String.valueOf(partID);
   }

    public Part getPart(){
        return part.getValue();
    }
    public void setPart(Part p){
        part.setValue(p);
    }
    public SimpleObjectProperty<Part> getPartProperty(){
        return part;
    }
    
    public void setDownloadMetadata(DownloadMetadata downloadMetadata){
        this.downloadMetadata=downloadMetadata;
    }
    
    public SimpleObjectProperty<DownloadStatus> getStatusProperty() {
        return status;
    }
    public DownloadStatus getStatus(){
        return status.getValue();
    }
    public void setStatus(DownloadStatus s) {
        status.setValue(s);
    }
   
   public void setCompletedBytes(long b){
       completedBytes.setValue(b);
   }
      
   public long getCompletedBytes(){
       return completedBytes.getValue();
   }
   
   public SimpleObjectProperty<Long> getCompletedBytesProperty(){
       return completedBytes;
   }
   
   public void setRetries(int r){
       retries.setValue(r);
   }
   
   public int getRetries(){
       return retries.getValue();
   }
   
   public void incrementRetries(){
       retries.setValue(retries.getValue()+1);
   }
   public SimpleObjectProperty<Integer> getRetriesProperty(){
       return retries;
   }

    public String getFilename() {
        return filename;
    }

    public void setFilename(String filename) {
        this.filename = filename;
    }
   
   
}

DownloadStatus.java

This file contains the enums of the DownloadStatus. Such as COMPLETED,PAUSED.

package downloadmanager;    

/**
 *
 * @author gnik
 */
public enum DownloadStatus{
    NEW,
    STARTING,
    DOWNLOADING,
    PAUSED,
    STOPPED,
    ERROR,
    JOINING,
    COMPLETED,

}

DownloadState.java

When the user wants to exit the program, a list of these objects is written to the disk to make the downloads persistent. The object contains DownloadMetadata and several DownloadPartMetadata objects. It essentially represents the state of the download at the end of the program.

package downloadmanager;

import java.util.List;

/**
 *
 * @author gnik
 */
public class DownloadState {
    public DownloadMetadata downloadMetadata;
    public List<DownloadPartMetadata> downloadPartMetadata;
    public DownloadState(){
        
    }
    public DownloadState(DownloadMetadata downloadMetadata, List<DownloadPartMetadata> downloadPartMetadata) {
        this.downloadMetadata = downloadMetadata;
        this.downloadPartMetadata = downloadPartMetadata;
    }

        
}

Part.java

This is just an object that contains the start and end byte of each DownloadPart. It is similar to Pair objects available in many languages (but not java).

package downloadmanager;


/**
 *
 * @author gnik
 */
public class Part{
    long startByte;
    long endByte;
    
    public Part(long startByte,long endByte){
        this.startByte=startByte;
        this.endByte=endByte;
    }

    public long getStartByte() {
        return startByte;
    }

    public long getEndByte() {
        return endByte;
    }

    public void setStartByte(long startByte) {
        this.startByte = startByte;
    }

    public void setEndByte(long endByte) {
        this.endByte = endByte;
    }
    
    @Override
    public String toString(){
        return String.valueOf(startByte)+"-"+String.valueOf(endByte);
    }
    
}

DownloadAction.java

This file contains the enums for the various action (and responses) a user can perform such as (Start a download/Pause a download). This is used to communicate in the queues with the thread.

package downloadmanager;

/**
 *
 * @author gnik
 */
public class DownloadAction {
    enum Command {
        STOP,
        RESUME,
        PAUSE
    }
    enum Response{
        STOPPED,
        RESUMED,
        PAUSED
    }
    
}

DownloadSaves.java

This class is responsible for writing and reading objects into the memory at the start and end of each program execution. A list of DownloadState objects is serialized in the disk to make the downloads persistent. It is loaded each time the program is run. Basically this is what stores the history of the class.

package downloadmanager;

import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.io.xml.StaxDriver;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
 *
 * @author gnik
 */
public class DownloadSaves {

    private List<DownloadState> downloads = new ArrayList<>();
    private final String saveFilename = "history.dat";

    public DownloadSaves() {

    }

    public void addDownload(DownloadState download) {
        downloads.add(download);
    }

    public List<DownloadState> getDownloads() {
        return downloads;
    }

    public void clear() {
        downloads = new ArrayList<>();

    }

    public void save() {
        XStream xstream = new XStream(new StaxDriver());
        String object = xstream.toXML(downloads);
        try (OutputStreamWriter file = new OutputStreamWriter(new FileOutputStream(saveFilename), StandardCharsets.UTF_8)) {
            file.write(object);
        } catch (IOException ex) {
            Logger.getLogger(DownloadSaves.class.getName()).log(Level.SEVERE, null, ex);
        }

    }

    public void createNewFile() {
        String object="<?xml version=\"1.0\" ?><list></list>";
        try (OutputStreamWriter file = new OutputStreamWriter(new FileOutputStream(saveFilename), StandardCharsets.UTF_8)) {
            file.write(object);
        } catch (IOException ex) {
            Logger.getLogger(DownloadSaves.class.getName()).log(Level.SEVERE, null, ex);
        }

    }

    public void load() {
        try (InputStreamReader reader = new InputStreamReader(new FileInputStream(saveFilename), StandardCharsets.UTF_8)) {

            XStream xstream = new XStream(new StaxDriver());
            downloads = (List<DownloadState>) xstream.fromXML(reader);
        } catch (FileNotFoundException ex) {
            Logger.getLogger(DownloadSaves.class.getName()).log(Level.SEVERE, null, ex);
            createNewFile();
        } catch (IOException ex) {
            Logger.getLogger(DownloadSaves.class.getName()).log(Level.SEVERE, null, ex);
        }

    }
}
\$\endgroup\$
2
  • \$\begingroup\$ I would recommend that you add a short summary at the top of the overall design/structure of your code, just a short summary about what the classes do and interact, as you have multiple classes here. See also Simon's guide to asking a good question \$\endgroup\$ Commented Sep 1, 2018 at 10:32
  • \$\begingroup\$ @SimonForsberg Hope the additional info is enough. \$\endgroup\$
    – Gnik
    Commented Sep 1, 2018 at 11:19

1 Answer 1

2
\$\begingroup\$

Upfront, I've just grabbed a couple of things that stuck out to me, I've not dug too much into the details. Also while this got rather long, I think the code is pretty well readable (which is easily one of the most important things) and it also deals with a good number of edge cases that obviously come up with real-life use, so congrats on that. Btw. I do like the README, all information there plus a screenshot, that's all I want.


First thing I did when opening was to click every button and while there are no crashes (good) I'd rather not see exceptions in the console - in general it would be good idea to handle those invariants ("there needs to be a download (at all, selected, ...) for 'Pause' to make any sense").

Secondly, resizing of the UI elements could be nicer, but I don't know JavaFX at all, so I can't tell you how exactly to change that:

  • Resizing the window to a size larger than the default leaves a gap at the bottom that could be filled by the table instead.
  • Resizing it to a smaller size than expected hides some UI elements ("Download", "URL:", and all the buttons eventually). In particular I'd suggest that the text entry should be the one thing to get smaller and the whole window might benefit from a minimal size as well.

The smallest window:

enter image description here

A smaller window:

enter image description here

A too big window:

enter image description here

Third thing was to enter a random website's address, which resulted in another exception.

Okay, fourth try, a URL to a picture, this time things are happening, but it errors out ... right, you might want to show some information if an error occured, because I've no idea why it didn't that time, a few other pages do work fine.

Also generally I'd be really careful with having the remove button (any action really) remove files on disk, in particular when there's no dedicated download folder and the download is just writing to the local directory. Which brings me to the fact that it's writing to the local directory and basically overwriting existing files if the filename ends up the same - with only the ID being prefixed that's easily possible.

Other than it being a bit dangerous :) looks nice and the UI feels smooth.


Now to the code - I'm not going to mention again what I said above, it should be relatively clear for you where those points are located in the code.

  • There are a few comments from the IDE still in there, would be cleaner to remove them ("To change this license header, ...").

  • In Download.java, ranges != null && !ranges.equals("none") could be reversed to make it shorter, !"none".equals(ranges).

  • Still in that file, divideDownload, I can't tell if this is correctly dividing up the download parts at a glance, and I should be able to. Apart from a test I'd suggest not having double nor Math.round in there. Basically all but the last parts should really have the same size, so the multiplication is rather easy, the last part can either be done separately, or with a comparison to the full download size. Now that I think about it though, what about files of size zero to what, seven bytes? For correctness the code should handle those too (the number of parts is hardcoded to 8 though).

  • The idiom for loggers is usually private static final Logger logger = Logger.getLogger(...); on the class level, there's little point recreating a logger every time something needs to be logged.

  • In DownloadSaves.java I'd suggest to first open the file like you do, then directly stream the XML, the reader is already set up that way. From a quick glance at the documentation that's still called toXML, just with a second argument, the OutputStream. This is basically so that you avoid creating larger strings unnecessarily (which will matter if your structures are bigger than they are right now).

  • I'm also not quite sure if createNewFile is really necessary, ideally you should just be able to write out the empty list instead?
  • Oh now that I've looked at what it writes to disk ... that could also be smaller. I'm not sure how much benefit you're getting from serialising the state like that, so it's probably fine for the moment, just consider that at some point you might need to debug the content of that file, not to mention migrating it to a new version or so.

  • In Part.java I'd rather use "%d-%d".format(startByte, endByte) (c.f. Formatter instead of that long construction, but really anything that doesn't force you to call String.valueOf would be nicer.

  • In DownloadPart.java, copyToStream is pretty inefficient. Any time you see something copying files a single byte at a time that's probably not efficient. Of course you'd actually have to time it with a large number of samples, but I'd still suggest copying in larger chunks, say kilobytes to megabytes, is a better approach.

  • In DownloadPool.java, there's a null check on downloadSaves.getDownloads() - that indicates that the list should really always be there, non-nullably, so that the code get's easier. In fact if it was there the for-loop immediately after would still work, it just wouldn't do anything on the empty list.


Regarding the pattern of the command/response queues, it's probably safe since there's no locks or synchronized blocks to be seen, which is good. One thing though is that external methods access the queues of other classes and that definitely doesn't look good - I'd either suggest encapsulating the queues into a separate object ("channel", ...), or have better methods on each class. Also note that e.g. pauseAll is very synchronous: It sends "pause" to each thread after the other, but what it could do to speed it up (since every thread might take a while to shutdown) is to send "pause" to all threads, then wait for the responses from all threads. Also consider that stopDownload, pauseDownload and resumeDownload basically have the same structure, as have pauseAll, resumeAll and stopAll.

\$\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.