Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I need to create variable-length strings of dots/periods/full-stops to add to some text content, in a way that is similar to a formatted table-of-contents:

Chapter 1 .................................... 1
  Section 1.1 ................................ 1
  Subsection 1.1.2 .......................... 12

I know that things like String.format exist, but that is not suitable for formatting periods this way.

The code is complicated by the need for there to be many threads doing similar work at the same time.

I have written this utility class that caches a 'base' string in a Thread-local, and uses that to supply the required data.

Are there any alternatives, improvements, or other recommendations you have?

public final class DotPadding {

    private static final ThreadLocal<String> PAD_BASE = new ThreadLocal<String>() {
        @Override
        protected String initialValue() {
            return "................";
        }
    };

    public static final String getPadding(final int length) {
        if (length <= 0) {
            return "";
        }
        String base = PAD_BASE.get();
        if (base.length() >= length) {
            return base.substring(0, length);
        }
        while (base.length() < length) {
            base = base.concat(base);
        }
        PAD_BASE.set(base);
        return base.substring(0, length);
    }

}

As an example usage, this is one of the ways that the above code is used. There are other places too:

String description = getFormattedDescription(title);
String index = getFormattedIndex(page);
String line = description
            + " "
            + DotPadding.getPadding(width - (2 + description.length() + index.length()))
            + " "
            + index;
System.out.println(line);
share|improve this question
    
Shouldn't you have some sort of helper method like DotPadding.getPaddedString(String left, String right)? Or is that what the last code block is about? –  Bobby Mar 31 at 22:01
    
The way this code works is to just have a single length of dots: input value 3 will return ... and 5 will return ..... –  rolfl Mar 31 at 22:07
1  
No, what I meant is you're calculating the amount of dots needed for between description and index in the last code block, shouldn't that be wrapped in a helper method? –  Bobby Mar 31 at 22:10

2 Answers 2

up vote 4 down vote accepted

I find it amusing that you go to a lot of effort to make the dot generator efficient, with thread-local storage for caching. Then, when you actually need to use this string, you just concatenate strings the simple and inefficient way. You don't really gain any performance by caching the string of dots, since you'll be copying it anyway during the concatenation.

It would be nice to have a purpose-built object so that you could write:

Padding p = new Padding(48);
System.out.println(p.format("Chapter 1",           "1"));
System.out.println(p.format("  Section 1.1",       "1"));
System.out.println(p.format("  Subsection 1.1.2", "12"));

It would all be accomplished with one char[] array, and you should be able to use it normally without threading issues.

public class Padding {
    private char[] buf;
    private char leader, space;

    public Padding(int width) {
        this(width, '.');
    }

    public Padding(int width, char leader) {
        this(width, leader, ' ');
    }

    public Padding(int width, char leader, char space) {
        this.buf = new char[width];
        this.leader = leader;
        this.space = space;
    }

    public String format(String left, String right) {
        int lLen = left.length();
        int rLen = right.length();

        // Left and right text
        left.getChars(0, lLen, this.buf, 0);
        right.getChars(0, rLen, this.buf, this.buf.length - rLen);

        // Dot leader
        for (int i = lLen + 1; i < this.buf.length - rLen - 1; i++) {
            this.buf[i] = this.leader;
        }

        // Space padding
        this.buf[lLen] = this.buf[this.buf.length - rLen - 1] = this.space;

        return new String(this.buf);
    }
}
share|improve this answer
  1. Naive approach: use StringUtils.repeat (source) without the ThreadLocal:

    public static String repeat(final char ch, final int repeat) {
        final char[] buf = new char[repeat];
        for (int i = repeat - 1; i >= 0; i--) {
            buf[i] = ch;
        }
        return new String(buf);
    }
    

    Anyway, I suppose you did some profiling and it turned out that this is a bottleneck. Otherwise I'd really not complicate it with the ThreadLocal nor the doubling/concatenation logic.

  2. substring in Java 6 used the same character array as the original String instance but it was changed in Java 7. So, substring calls copies the character array anyway, therefore string doubling and calling substring does not seem beneficial to me. (Anyway, it should be measured.)

    There is one exception (according to the source which Eclipse brings up for Java 8) when beginIndex is zero and endIndex is the length of the original string. In this case substring returns the same String instance without copying. This case seems unlikely here, I guess there's a little chance that PAD_BASE.lenght will be the same as the length parameter since the while loop doubles the size of the padding string on every iteration.

    So, in an average case the naive approach would create less new String (and underlying char[]) objects here than the one with substring.

  3. Another idea is storing an array of strings in the ThreadLocal and extending the array on demand if the required lenght is longer than paddings.lenght:

    String paddings[] = new String[max];
    for (int i = 0; i < max; i++) {
        paddings[i] = StringUtils.repeat('.', i);
    }
    
  4. 2 could be a named constant.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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