HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Check string value of boolean to a mapped enum

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mappedbooleanenumvaluecheckstring

Problem

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?

Solution

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.

Code Snippets

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

!(false || false) -> !false -> true
Boolean.parseBoolean(value) || "1".equals(value)
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);
}

Context

StackExchange Code Review Q#159235, answer score: 2

Revisions (0)

No revisions yet.