3
\$\begingroup\$

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:

ProductTextHelper.getTextInOrder(TextType.usecase2, product);
\$\endgroup\$
3
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ I really need to read Effective Java properly. Thanks! And I thought about the Unmodifiable List myself while simplifying the code. Thanks again :) \$\endgroup\$ – CWagner Mar 13 '13 at 15:49
  • \$\begingroup\$ I did not know it was in the book either. I googled it. :)) \$\endgroup\$ – abuzittin gillifirca Mar 13 '13 at 15:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.