How to avoid code smells (too many if else) where each enum if condition will call different methods which are controlled by the third party library?

Question

(Understanding OpenSearch/ElasticSearch is not important for this question, but the below information provides some context). I am using the new opensearch-java (version 2.4.0) library to interact with the OpenSearch cluster. I am trying to build the TypeMapping using Builder pattern, so that I can pass these properties mapping when creating a new index in OpenSearch like so:

// client will call this method when creating index.
public void createIndex(String name, TypeMapping typeMappings) {
    CreateIndexRequest createIndexRequest = new CreateIndexRequest.Builder()
                .index(name)
                .mappings(typeMappings)
                .build();
    createIndex(openSearchClient, createIndexRequest);
}
private static void createIndex(OpenSearchClient osClient, CreateIndexRequest createIndexRequest) {
    osClient.indices().create(createIndexRequest);
}

I have the following code to use the Builder pattern to build the TypeMapping.

package test;

import java.util.Map;
import org.opensearch.client.opensearch._types.mapping.Property;
import org.opensearch.client.opensearch._types.mapping.TypeMapping;
import org.opensearch.client.opensearch._types.mapping.DynamicMapping;

 
public class TypeMappingBuilder {
    private DynamicMapping dynamic;
    private final Map<String, Property> properties;

    public TypeMappingBuilder() {
        this.properties = new HashMap<>();
    }

    public TypeMappingBuilder disableDynamic() {
        this.mapping = DynamicMapping.Strict;
        return this;
    }
    
    public TypeMappingBuilder putTypeProperty(
            String name, 
            Property.Kind type, 
            boolean shouldIndex, 
            Map<String, Property> subTypeProperties) {
        this.properties.put(name, getTypeProperty(type, shouldIndex, subTypeProperties));
        return this;
    }

    private Property getTypeProperty(
            Property.Kind type, 
            boolean shouldIndex, 
            Map<String, Property> subTypeProperties) {
        if (type == Property.Kind.Text) {
// Here, in (p -> p), p is the TextProperty.Builder() provided by OpenSearch Java client
            return new Property.Builder().text(p -> p.index(shouldIndex).fields(subTypeProperties)).build();
        }
        if (type == Property.Kind.Keyword) {
// Here, in (p -> p), p is the KeywordProperty.Builder() provided by OpenSearch Java client
            return new Property.Builder().keyword(p -> p.index(shouldIndex).fields(subTypeProperties)).build();
        }
//...
//...
//... and so forth, there are more than 20 of the Property.Kind values where I can use the if condition
    }

    public TypeMapping build() {
        return new TypeMapping.Builder()
                .dynamic(dynamic)
                .properties(properties)
                .build();
    }
}

The issue is that too many if statement here (in method getTypeProperty) will cause code smell and also if a new type is added in the future, then I have to update getTypeProperty method to apply for the new type. Also, in this new Property.Builder().{specificType}({specificTypeBuilder}).build(), I cannot use polymorphism as there is different method names for each {specificType} and different type Builder for each {specificTypeBuilder}. I don't see any other options than to use if statements to create different Property based on a specific type. I thought of using static Map like so and access specific Property from the Map, but would only work if I didn't have subTypeProperties to put in for sub fields (but, it will have more memory and type overhead):

Map<PropertyHolder, Property> propertyMap = ImmutableMap.<PropertyHolder, Property>builder()
            .put(new PropertyHolder(Property.Kind.Text, true), new Property.Builder()
                    .text(p -> p.index(true)).build())
            .put(new PropertyHolder(Property.Kind.Text, false), new Property.Builder()
                    .text(p -> p.index(false)).build())
            .put(new PropertyHolder(Property.Kind.Keyword, true), new Property.Builder()
                    .keyword(p -> p.index(true)).build())
            .put(new PropertyHolder(Property.Kind.Keyword, false), new Property.Builder()
                    .keyword(p -> p.index(false)).build())
            ...
            ...
            ...
            .build();

    final class PropertyHolder {
        private final Property.Kind type;
        private final boolean index;

        private PropertyHolder(Property.Kind type, boolean index) {
            this.type = type;
            this.index = index;
        }

        public static Property getTypeProperty(Property.Kind type, boolean shouldIndex) {
            return propertyMap.get(new PropertyHolder(type, shouldIndex));
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            PropertyHolder that = (PropertyHolder) o;
            return type == that.type && index == that.index;
        }

        @Override
        public int hashCode() {
            return Objects.hash(type, index);
        }
    }

Another option that can actually work was to create my own enum and have a method defined for each enum value that returns a specific Property based on the enum value (Strategy pattern). But, that would be an abuse of enums and probably code smell. Here is what I am talking about:

public enum Type {
        TEXT {
            @Override
            public Property getTypeProperty(boolean shouldIndex, Map<String, Property> subTypeProperties) {
                return new Property.Builder().text(p -> p.index(shouldIndex).fields(subTypeProperties)).build();
            }
        },
        KEYWORD {
            @Override
            public Property getTypeProperty(boolean shouldIndex, Map<String, Property> subTypeProperties) {
                return new Property.Builder().keyword(p -> p.index(shouldIndex).fields(subTypeProperties)).build();
            }
        },
        ...,
        ...,
        ...;

        public abstract Property getTypeProperty(boolean shouldIndex, Map<String, Property> subTypeProperties);
    }

What design pattern can I use in this scenario to avoid code smell? Thank you!

EDIT:

In the scenario that I am working on, the client can either create TypeMapping using TypeMappingBuilder and then, pass that to the createIndex request, like so (added based on the answer mentioned by BionicCode):

TypeMapping typeMappings = new TypeMappingBuilder();
builder.putTextProperty(...)
    .putTextProperty(...)
    .putKeywordProperty(...)
    .build();
createIndex(indexName, typeMappings);

or using MappingFactory like so:

private static final Map<String, Property> MAPPINGS = ImmutableMap.<String, Property>builder()
        .putAll(MappingFactory.getMapping(Memory.class))
        .putAll(MappingFactory.getMapping(Cpu.class))
...
...
...
        .build();
TypeMapping typeMappings = new TypeMapping.Builder().properties(MAPPINGS).build();
createIndex(indexName, typeMappings);

Here, the Memory.class or Cpu.class POJO looks like this:

class Memory {
    @TypeAnnotation(name = "mem_type", type = Type.KEYWORD, index = FALSE)
    private final String memType;

    @TypeAnnotation(name = "bytes", type = Type.LONG, index = true)
    private final long bytes;
...
...
...
}

And, the TypeAnnotation in my library looks like this:

@interface TypeAnnotation {
    String name();
    Type type();
    boolean index();

    public static final Map<Type, Factory> TYPE_FACTORY_MAP = ImmutableMap.<Type, Factory>builder()
        .put(Type.TEXT, new TextFactory())
        .put(Type.KEYWORD, new KeywordFactory())
...
...
...
        .build();
}

Now, in the scenario where the Client uses TypeMappingBuilder, I can setup putKeywordProperty, putTextProperty, etc. methods for the client to use to add specific property to the properties Map before building TypeMapping. But, when the Client calls MappingFactory.getMapping(Memory.class), the current implementation for getMapping(Class) method is like so:

class MappingFactory {
    public static Map<String, Property> getMapping(Class<?> clazz) {
        Walker walker = new Walker();
        // this will walk the fields and add each Property to Walker properties Map. 
        ReflectionUtils.doWithFields(clazz, walker);
        return walker.getMapping();
    }

    // FieldCallback is provided by Spring framework as part of it's ReflectionUtils
    private static final class Walker implements FieldCallback {
        private final ImmutableMap.Builder<String, Property> properties = ImmutableMap.builder();
        public ImmutableMap<String, Property> getMapping() {
            return this.properties.build();
        }

        @Override
        public void doWith(java.lang.reflect.Field field) {
            TypeAnnotation typeAnnotation = (TypeAnnotation) field.getAnnotation(TypeAnnotation.class);
            String name = typeAnnotation.name();
            Type type = typeAnnotation.type();
            boolean index = typeAnnotation.index();
            // map defined in TypeAnnotation that contains <Type, Factory> and will return a specific property Factory (implements Factory) that you can use to create Property.
            Factory factory = TYPE_FACTORY_MAP.get(type);
            Property property = factory.getProperty(index);
            this.properties.put(name, property);
        }
    }
}
interface Factory {
    Property getProperty(boolean index);
    Property getProperty(boolean index, Map<String, Property> subProperties);
}
class KeywordFactory implements Factory {
    @Override
    public Property getProperty(boolean index) {
        return new Property.Builder().keyword(p -> p.index(index)).build();
    }

    @Override
    public Property getProperty(boolean index, Map<String, Property> subProperties) {
        return new Property.Builder().keyword(p -> p.index(index).fields(subProperties)).build();
    }
}

The issue is that I have to use either if/else or switch or factory pattern (I am using factory pattern here in doWith method and storing each factory in a Map<Type, Factory> that is defined in TypeAnnotation). Is there any other solution or a solution that is better when the client have the options to either use TypeMappingBuilder or MappingFactory? Thanks!

Answer 1

Usually, the caller of your API knows all the details of the context. You only have to expose a complete path for each supported context.
As a result the client code now explicitly selects the method/execution path instead of selecting an enum value.
This will make your API bigger opposed to a more generic API. But if it is about code paths, generic APIs are not useful (see your problem).
generic APIs only help in terms of type compatibility (input/output).

In terms of execution paths, you always need the client of your API to select an execution path: either in form of a parameter (you current choice) or by explicitly invoking the desired functionality.
The latter provides:

• a much cleaner API that allows the client code to communicates the intend better: it's more convenient to read and understand the client code because we are now enabled to read method names opposed to having to read through an argument list. Argument names are only useful if the reader knows about their purpose. To know this he must read the API documentation. Method names on the other hand directly communicate the purpose of the operation without the requirement to read any documentation.
• it also reduces the parameter count of the method: in your case the enum parameter is converted to a method name.
• and as a bonus you avoid extra documentation: the method names now clearly communicate the purpose of the method (if the names are chosen properly).
• you eliminate the Property.Kind enum (additional type only introduced to select the execution path).
• you eliminate the terrible to maintain cascades of if-statements and switch cases: this code breaks the Open-Close principle and therefore does not make your code/class extensible.

The solution is to allow the builder to explicitly fork the execution path. This eliminates the Property.Kind enum (previously used to identify the selected execution path) and the getTypeProperty method (previously used to execute the specialized execution path). Now the client of the TypeMappingBuilder API is responsible to select the execution path explicitly by calling the related method instead of passing the enum identifier.

The result is a more verbose client code (in terms of enhanced readability) and a more convenient API.
It's the only solution that improves the API, for example in contrast to alternative solutions like looking up instances in a hash table.
A hash table solution only works for object lookup. A hash table still won't solve more complex scenarios. In addition a hash table solution adds more memory and types overhead - while we usually can live with the memory overhead, the additional types we have to implement add unnecessary complexity).

So instead of adding more complexity to your internal code, simply clean up the API:

Usage example:

// Easier to read client code, where the intend is communicated via method names instead of argument names. 
// Even the argument name is meaning less 
// if the reader does not know that the argument is an actual execution path selector.
// To know this, he would have to read the API documentation.
// The method name allows the reader to instantly know the purpose 
// and the author which method to invoke.
TypeMappingBuilder builder = new TypeMappingBuilder();
builder.putTextProperty(...)
  .putTextProperty(...)
  .putKeywordProperty(...);

TypeMappingBuilder.java

public class TypeMappingBuilder {
    private DynamicMapping dynamic;
    private final Map<String, Property> properties;

    public TypeMappingBuilder() {
        this.properties = new HashMap<>();
    }

    public TypeMappingBuilder disableDynamic() {
        this.mapping = DynamicMapping.Strict;
        return this;
    }
    
    public TypeMappingBuilder putTextProperty(
            String name, 
            boolean shouldIndex, 
            Map<String, Property> subTypeProperties) {

        Property textProperty = new Property.Builder().text(p -> p.index(shouldIndex).fields(subTypeProperties)).build();
        this.properties.put(name, textProperty);

        return this;
    }
    
    public TypeMappingBuilder putKeywordProperty(
            String name, 
            boolean shouldIndex, 
            Map<String, Property> subTypeProperties) {

        Property keywordProperty = new Property.Builder().keyword(p -> p.index(shouldIndex).fields(subTypeProperties)).build();
        this.properties.put(name, keywordProperty);

        return this;
    }

    public TypeMapping build() {
        return new TypeMapping.Builder()
                .dynamic(dynamic)
                .properties(properties)
                .build();
    }
}