mercredi 26 juillet 2023

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

(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.

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 using too many if statement here 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:

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!

Aucun commentaire:

Enregistrer un commentaire