HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Method for downloading a PDF from a website in Android

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
websitemethodandroiddownloadingforfrompdf

Problem

How is this? Any ways to improve?

public void DownloadPDF(Uri uri) {
    downloadProgress.setVisibility(View.VISIBLE);
    downloadComplete.setVisibility(View.INVISIBLE);
    openFile.setText("Wait..."); // My button
    openFile.setEnabled(false);
    if (uri != null) {
        DownloadManager.Request request = new DownloadManager.Request(uri);
        request.setTitle(pdf + ".pdf");
        request.setDescription("Website: " + pdf);
        request.setMimeType("application/pdf");
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB) {
            request.allowScanningByMediaScanner();
            request.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED);
        }
        request.setDestinationInExternalPublicDir(Environment.DIRECTORY_DOWNLOADS + "/SavedPDFs/", pdf + ".pdf");
        manager = (DownloadManager) getSystemService(Context.DOWNLOAD_SERVICE);
        downloadId = manager.enqueue(request);
        onComplete = new BroadcastReceiver() {
            public void onReceive(Context ctxt, Intent intent) {
                try {
                    unregisterReceiver(onComplete);
                } catch(IllegalArgumentException e){
                    e.printStackTrace();
                }
                downloadComplete.setVisibility(View.VISIBLE);
                downloadProgress.setVisibility(View.INVISIBLE);
                downloadStatus.setText("Download Complete!");
                updateNumberOfFiles();
                openFile.setText("Open!");
                openFile.setEnabled(true);
            }
        };
        registerReceiver(onComplete, new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE));
    }
}

Solution

Android's method naming convention follows that of Java, so your method name DownloadPDF() will probably be better written as downloadPDF() (or maybe even downloadPdf()).

Your null check for uri comes in after you have toggled your UI elements, wouldn't that mean the state is possibly inconsistent if the method really received a null argument, and then the UL elements already indicated the code is downloading something? You may want to return from your method first in this case...

public void downloadPdf(URL uri) {
    if (uri == null) {
        return;
    }
    // ...
}


DownloadManager.Request methods return the object itself, so you may also consider daisy-chaining the setters together:

request.setTitle(pdf + ".pdf")
    .setDescription("Website: " + pdf)
    .setMimeType("application/pdf");

Code Snippets

public void downloadPdf(URL uri) {
    if (uri == null) {
        return;
    }
    // ...
}
request.setTitle(pdf + ".pdf")
    .setDescription("Website: " + pdf)
    .setMimeType("application/pdf");

Context

StackExchange Code Review Q#98086, answer score: 4

Revisions (0)

No revisions yet.