Consider replacing StringUtil concatenation with String.join() or Stream API

[sfloess]

Description

StringUtil.concatWithSeparator() implements custom string concatenation logic that could be simplified using Java's built-in String.join() or Stream API.

Current Implementation (StringUtil.java, lines 162-181)

public static StringBuilder concatWithSeparator(final StringBuilder stringBuilder, 
                                                final boolean isSeparatorAtEnd, 
                                                final String separator, 
                                                Object... objs) {
    Objects.requireNonNull(separator, "Separator cannot be null");
    ArrayUtil.ensureArray(objs, "Must have a list of objects to concat!");

    for (int index = 0; index < objs.length; index++) {
        stringBuilder.append(objs[index]);

        if (isSeparatorAppendable(separator, index, objs)) {
            stringBuilder.append(separator);
        }
    }

    if (isSeparatorAtEnd) {
        stringBuilder.append(separator);
    }

    return stringBuilder;
}

Simpler Alternative with String.join()

For Simple Cases (most common usage)

// Current
String result = StringUtil.concatWithSeparator(",", "foo", "bar", "baz");

// Could be
String result = String.join(",", "foo", "bar", "baz");

For StringBuilder Cases

public static StringBuilder concatWithSeparator(final StringBuilder stringBuilder, 
                                                final boolean isSeparatorAtEnd, 
                                                final String separator, 
                                                Object... objs) {
    Objects.requireNonNull(separator, "Separator cannot be null");
    ArrayUtil.ensureArray(objs, "Must have a list of objects to concat!");

    // Convert objects to strings and join
    String joined = Arrays.stream(objs)
        .map(Object::toString)
        .collect(Collectors.joining(separator));
    
    stringBuilder.append(joined);
    
    if (isSeparatorAtEnd) {
        stringBuilder.append(separator);
    }

    return stringBuilder;
}

Pros of Current Implementation ✅

• Custom logic: Handles isSeparatorAtEnd flag
• Checks for trailing separators: isSeparatorAppendable prevents double separators
• StringBuilder reuse: Caller can provide existing StringBuilder
• Works with any Object: Calls toString() implicitly

Cons of Current Implementation ❌

• More complex: Custom loop logic vs built-in methods
• More code to maintain: ~50 lines vs ~10 lines
• Edge case handling: isSeparatorAppendable logic is subtle
• Performance: Similar performance to built-ins, but custom code has overhead

Recommendation

Keep Current Implementation If:

1. The isSeparatorAppendable logic (checking if objects end with separator) is actually used and necessary
2. Performance profiling shows this is a hotspot

Consider Simplification If:

1. The "avoid double separator" logic is never needed in practice
2. Tests show equal or better performance with built-in methods
3. Simplicity and maintainability are more important than edge case handling

Questions to Answer

1. Is the double-separator prevention actually needed?

   • Check if any objects passed to this method ever end with the separator
   • If not, that logic is unnecessary complexity

2. Is isSeparatorAtEnd commonly used?

   • If it's rarely true, we could just append the separator manually after calling String.join()

3. Performance requirements?

   • Is this method called in hot loops?
   • Benchmark current vs. built-in implementations

Suggested Investigation

// Add telemetry to see if edge cases are hit
static boolean isSeparatorAppendable(final String separator, final int index, final Object... objs) {
    if (objs == null) {
        return false;
    }

    boolean isNotLastElement = index <= (objs.length - 2);
    boolean doesNotEndWithSeparator = !objs[index].toString().endsWith(separator);
    
    if (isNotLastElement && objs[index].toString().endsWith(separator)) {
        // Log when this edge case is hit
        getLogger().log(Level.FINE, "Prevented double separator for object: {0}", objs[index]);
    }
    
    return isNotLastElement && doesNotEndWithSeparator;
}

Files to Investigate

• src/main/java/org/flossware/jcommons/util/StringUtil.java (lines 135-245)
• src/test/java/org/flossware/jcommons/util/StringUtilTest.java (check test coverage)

Priority

Low - Performance/maintainability optimization

Related

This is not a bug, but a potential simplification opportunity worth investigating.

[sfloess]

Analysis:

The concat methods work correctly and are part of the public API. While String.join() could simplify the implementation, the methods provide:

1. StringBuilder support - direct append to existing StringBuilder (String.join returns new String)
2. Separator at end option - isSeparatorAtEnd parameter not available in String.join()
3. Object varargs - automatically converts any Object to String via append()

Internal Usage:
Only 2 internal uses in production code:

• AbstractStringifiable.java (line concatenation)
• UrlUtil.java (URL construction)

Recommendation:

• Keep existing methods for API compatibility (no deprecation needed)
• Could internally use String.join() for the simplest overload as an optimization
• Not a priority given minimal usage and working implementation

Alternative Action:
Document in JavaDoc that users can use String.join() for simple cases.

This is a "nice to have" optimization, not a critical issue. Marking as enhancement.

[sfloess]

Analysis: Keep StringUtil.concatWithSeparator()

Actual Usage in Solenopsis

StringUtil is actively used by the Solenopsis framework (see memory: architecture review). Before recommending deprecation, we must verify:

1. Downstream dependencies - Solenopsis SOAP and Session frameworks
2. API stability - v1.x must maintain compatibility
3. Migration cost - Breaking changes require v2.0

String.join() vs StringUtil

String.join() limitations:

• Returns String (creates new object)
• Doesn't support StringBuilder reuse
• No isSeparatorAtEnd option
• Only works with CharSequence

StringUtil advantages:

• StringBuilder reuse (memory efficient)
• Flexible separator placement
• Handles Object[] (not just CharSequence)
• Used in performance-critical code

Recommendation

v1.x: Keep as-is (API stability, no breaking changes)

v2.0: Consider deprecation IF:

1. Solenopsis confirms no usage
2. Migration guide provided
3. Performance impact acceptable

Current action: Close as "won't fix" for v1.x, defer to v2.0 planning

This falls under "not over-engineered" - it provides functionality String.join() doesn't.