CODEWITHSUNDEEP
javaif-statementjava-8refactoringoption-type
AnOldSoul
AnOldSoul

Reputation: 4207

How to optimize the if-else statement due to Optional Object's properties also being optional?

I have an Optional Object named Form. This has 3 other optional properties to it. Reason being, if this form object is not present, I have to generate my own form data. If the form is present, users can still choose to leave their properties empty, requiring my method to generate them in that case. The way that I have implemented this logic goes as follows.

private FormSection buildForm(final Optional<Form> formOptional) {
    Set<String> formNames = null;
    Set<Question> formTypes = null;
    Set<Question> formDetails = null;

    if (formOptional.isPresent()) {
        Form form = formOptional.get();
        formNames = formSection.names()
                .orElseGet(() -> getNamesFromDB());
        formTypes = formSection.formTypes()
                .orElseGet(() -> getFormTypesFromDB());
        formDetails = formSection.formDetails()
                .orElseGet(() -> getFormDetailsFromDB());
    } else {
        formNames = getNamesFromDB();
        formTypes = getFormTypesFromDB();
        formDetails = getFormDetailsFromDB();
    }
    return Form()
            .names(formNames)
            .formTypes(formTypes)
            .formDetails(formDetails)
            .build();
}

I feel like I'm redoing some checks and that this whole if-else statement could be optimized. I'm using Java 8 by the way.

Upvotes: 2

Views: 202

Answers (2)

areus
areus

Reputation: 2947

You can use flatMap method from Optional to "chain" operations that return another Optional

formNames = form.flatMap(Form::names).orElseGet(() -> getNamesFromDB());

flatMap gets applied only if form is not empty, otherwise returns an empty optional. If it's present, is replaced by the optional returned by names() method in Form.

private FormSection buildForm(final Optional<Form> opForm) {

    Set<String> formNames = opForm.flatMap(Form::names).orElseGet(() -> getNamesFromDB());
    Set<Question> formTypes = opForm.flatMap(Form::formTypes).orElseGet(() -> getFormTypesFromDB());
    Set<Question> formDetails = opForm.flatMap(Form::formDetails).orElseGet(() -> getFormDetailsFromDB());

    return Form()
            .names(formNames)
            .formTypes(formTypes)
            .formDetails(formDetails)
            .build();
}

Upvotes: 0

rzwitserloot
rzwitserloot

Reputation: 102902

Make a form impl that is entirely blank. Then strongly reconsider your APIs; Optional is an appropriate thing to return when you're writing stream terminals. It's borderline okay to return them for methods (but not idiomatic). It's almost universally considered 'wrong' to use them anywhere else (such as fields, or as a parameter type); see for example this top answer (Brian Goetz was team lead for Lambdas at oracle) that tells you not to do this.

The preferred solution to avoid nulls is to have sensible defaults if possible. Not all object kinds have these (and don't force it), but given that you have a form that has 100% optional data, it is appropriate here: Is there a semantic difference between a 'null' form (or a non-present optional, if you prefer), and a present/non-null form where every single thing the form contains is null/non-present?

I don't think you intend for there to be a difference. In which case - great. Ditch optional forms. Adopt the default form. You can then just pass the 'default form' (the one where all 'fields' are non-present) to this method:

class Form {
    public static final Form BLANK_FORM = ....;
}

private FormSection buildForm(Form form) {
}

and adjust whatever code currently ends up with an Optional<Form> to instead shift to returning a blank form. For example, instead of:

Optional<Form> formOptional = Optional.ofNullable(x);

use:

Form form = x == null ? Form.BLANK_FORM : x;

Or if you have:

if (....) {
    return Optional.empty();
} else {
    // fetch all sorts of details.
    Form f = new Form(...);
    return Optional.of(f);
}

replace those return statements with respectively return Form.BLANK_FORM; and return f;. You get the idea :)

Upvotes: 3

Related Questions