Tag: logic

My colleague showed me this piece of code, and we both wondered why there seemed like too much code:

private List<Foo> parseResponse(Response<ByteString> response) {
    if (response.status().code() != Status.OK.code() || !response.payload().isPresent()) {
      if (response.status().code() != Status.NOT_FOUND.code() || !response.payload().isPresent()) {
        LOG.error("Cannot fetch recently played, got status code {}", response.status());
      }
      return Lists.newArrayList();
    }
    // ...
    // ...
    // ...
    return someOtherList;
}

Here’s an alternate representation, to make it a little less wordy:

private void f() {
    if (S != 200 || !P) {
        if (S != 404 || !P) {
            Log();
        }
        return;
    }
    // ...
    // ...
    // ...
    return;
}

Is there a simpler way to write this, without duplicating the !P? If not, is there some unique property about the situation or conditions that makes it impossible to factor out the !P?

Solution:

Unfortunately, the only way I can see to condense the if-statement would be in removing braces as you have single lines.

if (S != 200 || !P) {
    if (S != 404 || !P) Log();
    return A;
}
return B;

If, however, you only had one branch reachable through this statement which was the Log(); statement, you could use the following logical identity to condense the logic (Distributive).

(S != 200 || !P) && (S! = 404 || !P) <=> (S != 200 && S != 404) || !P

If you wanted to only check the condition of truth value of !P once and maintain the same logical outcome, the following would be appropriate.

if (!P) {
    Log();
    return A;
}
if (S != 200) {
    if (S != 404) Log();
    return A;
}
return B;

Or

if (S == 404 && P) return A;
if (S != 200 || !P) {
    Log();
    return A;
}
return B;