Check string value of boolean to a mapped enum

Question

In a project, a boolean can be mapped to an enum to display a custom string for true and false. If I have a string interpretation of a value, I have a method that is checking against the custom enum names or/and against regular boolean expressions like true, false, 0 and 1.

The method looks like this

public Boolean createParameterObjectValue(String value) {
 if (getMetaData().getValidValues().isAvailable() && getMetaData().getValidValues().getType() == ValidValues.Type.ENUM) {
 if (getMetaData().getValidValues(EnumValidValues.class).getEnumName(1).equalsIgnoreCase(value)) {
 return true;
 }
 if (getMetaData().getValidValues(EnumValidValues.class).getEnumName(0).equalsIgnoreCase(value)) {
 return false;
 }
 //TODO refactor redundance
 return !("0".equals(value) || "FALSE".equalsIgnoreCase(value));
 } else {
 return !("0".equals(value) || "FALSE".equalsIgnoreCase(value));
 }
}

First I'll check if the object in my project (a parameter) that is handling the boolean value, has defined values and has an enum mapping.

If this is true, compare the value to the enum names. If none of them equals the given value,I assume that the given string is "0","1","false" or "true". The same happens if the parameter does not have an enum mapping.

As you see from the TODO comment I'm not really happy with the redundancy, even if it doesn't really affect the performance. How could I remove the redundancy from this code?

Answer 1

The redundant logical expression seems to be too complex for this case.

And there is probably a bug. You say that the expected set of values for it is ("0", "1", "false", "true"), but if value = "2", it will also evaluate to "true":

!("0".equals("2") || "FALSE".equalsIgnoreCase("2")); ->
!(false || false) -> !false -> true

"2" is weird for "true" value.

So I suggest to reformulate the expression to

Boolean.parseBoolean(value) || "1".equals(value)

"1" is still there, because parseBoolean checks for equivalence for "true" string value.

The logic of the entire method can be thus reformulated with improved readability and simplified:

public boolean createParameterObjectValue2(String value) {
 return Boolean.parseBoolean(value) ||
 "1".equals(value) ||
 evaluatesToTrueFromMetadata(value);
}
private boolean evaluatesToTrueFromMetadata(String value) {
 return getMetaData().getValidValues().isAvailable() &&
 getMetaData().getValidValues().getType() == ValidValues.Type.ENUM &&
 getMetaData().getValidValues(EnumValidValues.class).getEnumName(1).equalsIgnoreCase(value);
}

getMetaData().getValidValues() in the private method should also be extracted to a local reference in order to shorten the expression.

• Herr Derb: Yes it's on purpose that the function is testing on false. As per logic definition 0 is false, and therefor any other value has to be true.
• Antot: in this case you can keep the part !"0".equals(value).
• Herr Derb: But you are right. as Java goes the other way around it probably would make most sense go for the compromise !("0".equals(value)) || Boolean.parseBoolean(value);

Answer 2

Well it seems that I missed the wood for the trees...

The solution is rather simple. I'm able to remove the redundancy be just removing the else block... obviously.

public Boolean createParameterObjectValue(String value) {
 if (getMetaData().getValidValues().isAvailable() && getMetaData().getValidValues().getType() == ValidValues.Type.ENUM) {
 if (getMetaData().getValidValues(EnumValidValues.class).getEnumName(1).equalsIgnoreCase(value)) {
 return true;
 }
 if (getMetaData().getValidValues(EnumValidValues.class).getEnumName(0).equalsIgnoreCase(value)) {
 return false;
 }
 }
 return !("0".equals(value) || "FALSE".equalsIgnoreCase(value));
}