# Is my logic correct checking for a newer remote file?

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();

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...");
}

-

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.

-
String s = status.getMessage();
int i = status.getCode();
String e = s + " | Status Code: " + i;


can be directly rewrited :

String e = status.getMessage() + " | Status Code: " + status.getCode();


or more clean with Effective Java (Item 51)

StringBuilder sbTmp = new StringBuilder('MaxLenghtOfNormalMessages')
sbTmp.append(status.getMessage());
sbTmp.append( " | Status Code: ");
sbTmp.append(status.getCode());


Put also sbTmp in the else { .. } if it is not used outside it

-
I do not agree with the last point (replacing with StringBuilder). The item in effective java from bloch (not extreme java) mentions string rebuilding inside loops with lots of iterations. For the given example, the + operator is absolutely fine. It is more readable and will most probably be replaced by the interpreter/compiler to a StringBuilder anyway. This is an example of bad premature optimization. –  tb- Mar 6 '13 at 16:00
@tb- I do not know why I always change the title of Joshua Bloch book's, corrected. Perhaps StringBuilder looks luxury, but the main impact is given by the size of messages expected. The more efficient is to put it in the else : only build if necessary. –  cl-r Mar 6 '13 at 16:07