How can I simplify this set of if statements? (Or, what's making it feel so awkward?)?

Question

My colleague showed me this piece of code, and we both wondered why we couldn't seem to remove duplicated 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`?

Answer 1

No matter what the status code is, if isPresent is false, you're logging an error.

You're also logging an error when isPresent is true and the status code is neither 200 nor 404.

You're returning a newArrayList whenever isPresent is false and also when isPresent is true and the code is not 200.

if (!P || (S != 404 && S != 200))

{

Log();

return Lists.newArrayList();

}

else if (S == 404) // P and S==404

return Lists.newArrayList();

}

else // P and S == 200

{

// . . .

// . . .

// . . .

return someOtherList;

}