Sometimes a developer takes the shortest possible route to solve a problem even if it sacrifices readability. The following example requires some concentration to determine what the possible results are.
dlLocal = !"false".equalsIgnoreCase(worker.getConfig().getProperty(DOWNLOAD_LOCAL_SETTING));
The second example is wordier, but a junior developer will be able to understand it, and a senior developer will be able to understand it easily.
String configValue = worker.getConfig().getProperty(DOWNLOAD_LOCAL_SETTING);
if (configValue == null || configValue.equalsIgnoreCase(Boolean.TRUE.toString()))
{
downloadLocally = true;
}
else
{
downloadLocally = false; // we keep files on server.
}
The more concise version is just as acceptable:
String configValue = worker.getConfig().getProperty(DOWNLOAD_LOCAL_SETTING);
downloadLocally = (configValue == null || configValue.equalsIgnoreCase(Boolean.TRUE.toString()))
When readability is poor, maintainability is also poor:
postWithMediaList.setImages(new Media[imageBufferList.size()]);
imageList.toArray(postWithMediaList.getImages());
To the casual observer the above code appears to do something strange with imageList. But in reality it is using a getter to change the value of postWithMediaList. The real function of the code has become obscured because the programmer was trying to save one line of text. Suddenly bugs are harder to spot.
A better approach is to consider the most common way the toArray() method is used and replicate that.
Media[] tempMedia = new Media[imageList.size()];
imageList.toArray(tempMedia);
postWithMediaList.setImages(tempMedia);
Tip: If unsure, write the kind of code that everyone is used to seeing.
