Java conditional - How to format simple if statement with 3 scenarios? if structure advice-CodePudding

I'm having an issue where my code just feels messy and I need some help on how I could structure it better.

example:

if (object.getDescription() == Status.Expected && !logEvent.equals("Expected")) {
    System.out.println("Do nothing"); // ???
} else {
    status.setChangedBy(logEvent);
}

How can i format this if in a cleaner way? I want the changedBy method to be called in every case except when getDescription == Status.Expected and logEvent is not "Expected". But I don't want an empty if statement either.

An alternative is:

if (object.getDescription() == Status.Expected) {
   if (logEvent.equals("Expected")) {
         status.setChangedBy(logEvent);
   }
} else {
    status.setChangedBy(logEvent);
}

Both examples work. But neither examples "feels right". Is there any other solution I'm not seeing?

CodePudding user response：

Invert the condition by applying the ! operator to the whole thing. Then you can put the code in the if block instead of the else block.

if (!(object.getDescription() == Status.Expected && !logEvent.equals("Expected"))) {
    status.setChangedBy(logEvent);
}

You can further simplify this using De Morgan's Law:

if (object.getDescription() != Status.Expected || logEvent.equals("Expected")) {
    status.setChangedBy(logEvent);
}

You change the status when "the description is not expected or logEvent is expected"

CodePudding user response：

If I understand right, what you are looking for is that :

if (object.getDescription() != Status.Expected || logEvent.equals("Expected")) {
    status.setChangedBy(logEvent);
}

I took your original comparison and reversed it.

CodePudding user response：

I know that it takes more code, but I would go for this approach:

if (shouldChangeStatus(object, logEvent))
    status.setChangedBy(logEvent);
}

private boolean shouldChangeStatus(Object object, Object logEvent) {
    if (object.getDescription() != Status.Expected) {
        return true;
    }
    if (logEvent.equals("Expected")) {
        return true;
    }
    return false;
}

You can simplify it to something like that

if (shouldChangeStatus(object, logEvent))
    status.setChangedBy(logEvent);
}


private boolean shouldChangeStatus(Object object, Object logEvent) {
    if (object.getDescription() != Status.Expected) {
        return true;
    }
    return logEvent.equals("Expected");
}

It's the easiest way to maintain that code, also if you ever need that check in other place you can just change private to public/protected or whatever and just use it without a need to copy paste code

CodePudding user response：

Minor addition to the previous answers: it's good practice to put the constant string first in a comparison like this "Expected".equals(logEvent). This way if logEvent is null, it will not throw a NPE.