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

Enums with different methods, useful or abuse?

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

Problem

I'm wondering if the way I wrote our TextHelper class is a really a proper way to do it.

We have products that are interlinked in different ways and every product has multiple text fields that might be empty.

Now depending on where the text is supposed to show up, we want different texts.

A simple example would be:

On the product detail page it should show the shop specific text, if not available, the main description, if that is not available, the main description of it's parent product and the short description of the parents' subproducts.

So much for the motivation. I tried to shorten and simplify the code as much as I could. Is this a good way to use enums or should this be done completely different?

public class ProductTextHelper {
    public static String getTextInOrder(TextType[] order, IProduct p) {
        for (TextType t : order) {
            if (!t.getText(p).equals(""))
                return t.getText(p);
        }
    }

    public enum TextType {
        text1 {
            public String getText(IProduct p) {
                return p.getText1;
            }
        },
        text2 {
            public String getText(IProduct p) {
                String t = p.getText2;
                return t.equals("") ? "" : t + TextType.text1.getText(p);
            }
        },
        text3 {
            public String getText(IProduct p) {
                StringBuilder sb = new StringBuilder();
                sb.append(p.getText3());
                for (IProduct subP : p.getRelatedProducts()) {
                    sb.append(TextType.text2.getText(subP));
                }
            }
        },
        // more enums

        public static final TextType[] useCase1 = {text1, text2};
        public static final TextType[] useCase2 = {text4, text1, text3};
        public static final TextType[] useCase3 = {text3, text2};
        // a few more arrays

        public abstract String getText(IProduct p);
    }
}


And this gets called like that:

```

Solution

My answer to your main question is you gotta do what you gotta do. Just so you feel more comfortable; this pattern has a name strategy enum. It is even in Effective Java.

But I have a criticism for this bit:

public static final TextType[] useCase1 = {text1, text2};


Arrays are mutable. You should not implement class constants as final arrays. Use unmodifiable collections instead:

public static final List useCase1 = Collections.unmodifiableList(
         Arrays.asList(text1, text2));


Also each time you add another use case you should not need to modify this enum, that is by adding another useCase. Those should be moved to wherever they are used.

Code Snippets

public static final TextType[] useCase1 = {text1, text2};
public static final List<TextType> useCase1 = Collections.unmodifiableList(
         Arrays.asList(text1, text2));

Context

StackExchange Code Review Q#23851, answer score: 3

Revisions (0)

No revisions yet.