XML tags constants vs hardcoded values

I have to create an XML body for a web service call. I want to use:

private void createProductBody(Product product, StringBuilder body) {
    body.append(WSConstants.WS_PRODUCT_ID_START_TAG)
            .append(product.getId())
            .append(WSConstants.WS_PRODUCT_ID_END_TAG)
            .append(WSConstants.WS_SIZE_TEXT_START_TAG)
            .append(product.getSize())
            .append(WSConstants.WS_SIZE_TEXT_END_TAG);
}

But some colleagues state that this writing is hard to read and they want us to use this:

private void createProductBody(Product product, StringBuilder body) {
    body.append("").append(product.getId()).append("")
        .append("").append(product.getSize()).append("");
}

Which are the pro and cons for using hardcoded values?

They're right. While "" is hard to read itself (YMMV), the constants make it even worse.

Magical constants in a code are bad and should usually be extracted as you did. But if there are many of them and each gets used just once, it's counterproductive.

The best solution would be something like

new XmlBuilder(body)
    .start("ProductId").append(product.getId()).end()
    .start("Size").append(product.getSize()).end();

or some of the million existing XML tools. If the constants get used multiple times, then something like

new XmlBuilder(body)
    .start(WSConstants.WS_PRODUCT_ID).append(product.getId()).end()
    .start(WSConstants.WS_SIZE).append(product.getSize()).end();

makes more sense. Then you may consider using static imports to keep it shorter.

The whole XmlBuilder is rather trivial, it mostly delegates to the StringBuilder and remembers the opened tags using a stack. As stated by aetheria, it should also escape the appended values when needed.

For redundancy lovers, something like

new XmlBuilder(body)
    .start(WS_PRODUCT_ID).append(product.getId()).end(WS_PRODUCT_ID)
    .start(WS_SIZE).append(product.getSize()).end(WS_SIZE);

may be more "cool". Anyway, it's more XML-like (just don't ask me what's good for).

Note the inconsistency of "PRODUCT_ID" vs. "SIZE_TEXT", where the former means indeed "productId" while the latter means just "size". This should be fixed (like I did in my snippets).

StackExchange Code Review Q#72013, answer score: 13