7
\$\begingroup\$

Cardshifter is an online trading card game, made by a gang of Code Review community members. Cards can have a bunch of different, and sometimes complex effects, and descriptions for these are automatically generated if no custom one is given.

EIGHT IMMORTALS: Choose 1 at random: Summon 1 Life Tool to your Battlefield or Summon 1 Destruction Tool to your Battlefield

The effect description code is quite messy and hard to adapt, which is why I've started a refactoring effort to improve it. The class up for review here is part of this effort.

Design aims

  • The ability to act as a Strangler Vine and gradually replace what already exists. This is achieved through the text method that simply proxies its input straight to the string builder (which is what was earlier used).

  • I wanted the class to be very robust and still produce consistent output no matter what is thrown at it. This is largely because of the legacy code, which is inconsistent in some areas.

The code

/**
 * Build sentences using a convenient DSL. Sentences are capitalized and end with a period.
 */
class SentenceBuilder {

    private StringBuilder stringBuilder = new StringBuilder()

    static String build(@DelegatesTo(strategy = Closure.DELEGATE_FIRST, value = SentenceBuilder) Closure closure) {
        def builder = new SentenceBuilder()
        closure.delegate = builder
        closure.resolveStrategy = Closure.DELEGATE_FIRST
        closure.call()

        return builder.toString()
    }

    String toString() {
        String sentence = stringBuilder.toString()

        sentence = sentence.trim()
        if (sentence.length() > 0 && !sentence.endsWith('.')) {
            sentence = sentence + '.'
        }
        sentence = sentence.capitalize()

        return sentence
    }

    /**
     * Add some text.
     */
    void text(String string) {
        stringBuilder.append(string)
    }

    /**
     * Add a separator if not already present and it's not the start of the sentence.
     * @param sep The separator, e.g. ' ' or ', '.
     */
    void separator(String sep) {
        if (stringBuilder.length() > 0 && !stringBuilder.toString().endsWith(sep)) {
            stringBuilder.append(sep)
        }
    }

    /**
     * Add a list like 'egg, bacon and ham'.
     * @param conjunction Conjunction between the last two elements, e.g. 'and'.
     * @param elements A list of the elements, e.g. ['egg', 'bacon', 'ham'].
     */
    void list(String conjunction, List<String> elements) {
        if (elements.size() > 0) {
            stringBuilder.append(elements[0])

            if (elements.size() > 1) {
                for (int i = 1; i < elements.size() - 1; i++) {
                    separator(', ')
                    stringBuilder.append(elements[i])
                }

                separator(' ')
                stringBuilder.append(conjunction)
                separator(' ')
                stringBuilder.append(elements[elements.size() - 1])
            }
        }
    }

}

I don't really like the implementation of list - it doesn't look very groovy at all, but I didn't come up with a better way to handle all cases.

Usage example

def things = ['cats', 'crackers', 'computers']
def feeling = 'like '

SentenceBuilder.build {
    text 'i'
    separator ' '
    text feeling
    separator ' '
    list 'and', things
}

// I like cats, crackers and computers.
\$\endgroup\$
1
\$\begingroup\$

Here's another way to implement the list() method:

switch(elements.size()) {
    case 0:
        null
        break
    case 1: 
        elements[0]
        break
    case 2:
        elements.join(" $conjunction ")
        break
    default:
        new StringBuilder().with {
            append elements.take(elements.size() - 1).join(', ')
            append " $conjunction "
            append elements.last()
        }.toString()
        break
}

This makes use of join() to handle most of the String building, instead of looping and appending to StringBuilder. The StringBuilder only comes in during the case of 3 or more items.

Revised

It's possible to knock out one of the cases like this:

switch(elements.size()) {
    case 0:
        ''
        break
    case 1: 
        elements[0]
        break
    default:
        new StringBuilder().with {
            append elements.take(Math.max(elements.size() - 1, 1)).join(', ')
            append " $conjunction "
            append elements.last()
        }.toString()
        break
}

So now there are cases for an empty list, a one-item list, and more-than one-item list.

\$\endgroup\$

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.