4
\$\begingroup\$

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));
    }
}
\$\endgroup\$
4
\$\begingroup\$

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");
\$\endgroup\$
  • \$\begingroup\$ Thanks, how would I add a progress bar to this download method? Also, am I handling the onComplete properly? \$\endgroup\$ – Faraz Jul 26 '15 at 4:53
  • \$\begingroup\$ @Faraz unfortunately I do not know much about Android development beyond Java... \$\endgroup\$ – h.j.k. Jul 26 '15 at 6:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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