patternjavaMinor

Is my logic correct checking for a newer remote file?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

filecheckingnewerlogicforcorrectremote

Problem

String slm = status.getHeader("last-modified");
        SimpleDateFormat sdf = new SimpleDateFormat(
                "EEE, dd MMM yyy HH:mm:ss zzz");

        Date serverLastMod = null;
        try {
            serverLastMod = sdf.parse(slm);
        } catch (ParseException e) {
            Log.e("SAC XML last-modified", e.getMessage());
        }
        Date fileLastFetched = status.getTime();

        Log.e("DATE1", serverLastMod.toGMTString());
        Log.e("DATE2", fileLastFetched.toGMTString());

        //if the server date is before my file date, update it, else dont

        if (serverLastMod.after(fileLastFetched)) {
            Log.e("DS", "YES! Getting ROSE");

            XmlDom r = xml.tag("reportrose");
            roseUrl = r.tag("img").attr("src").trim();
            aq.download(roseUrl, file, new AjaxCallback() {

                public void callback(String url, File file, AjaxStatus status) {
                    String s = status.getMessage();
                    int i = status.getCode();
                    String e = s + " | Status Code: " + i;

                    if (file != null) {
                        getInfo();
                    } else {
                        Log.e("ACR SAC Widget Error", e);
                    }
                }

            });
        } else {
            Log.e("DS", "NO! Don't do anything... Wait until next check...");
        }

Solution

Well, if it works, it works. There is not much to say about this small part of the code without any context. You should know this by yourself, because you should have some tests.

For the code:

The first catch is not suitable. If it happens, you will get a NullPointerException for the following if.

Avoid abbreviations. There are a lot of bad examples in this code, the most noticeable one is String e = s + " | Status Code: " + i;

Reduce indentation levels:

if (serverLastMod.after(fileLastFetched)) {
...
} else {
    Log.e("DS", "NO! Don't do anything... Wait until next check...");
}

could be:

if (!serverLastMod.after(fileLastFetched)) // or last <= current
{
    log...
    return
}
... rest of code...

and so on.

The part ...rest of code... could probably have its own method update..., so the purpose is clear.

Code Snippets

if (serverLastMod.after(fileLastFetched)) {
...
} else {
    Log.e("DS", "NO! Don't do anything... Wait until next check...");
}

if (!serverLastMod.after(fileLastFetched)) // or last <= current
{
    log...
    return
}
... rest of code...

Context

StackExchange Code Review Q#23512, answer score: 3

Revisions (0)

No revisions yet.