current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 2 down vote favorite

I have these two methods:

public static String buildLine(String[] values) {
    StringBuilder lineBuilder = new StringBuilder();
    for (int i = 0; i < values.length - 1; i++) {
        lineBuilder.append(values[i]).append(SEPARATOR);
    }
    lineBuilder.append(values[values.length - 1]);
    return lineBuilder.toString();
}

public static String buildLineFromNodeWorker(String[] values, NodeDecorator nodeWorker) {
    StringBuilder lineBuilder = new StringBuilder();
    for (int i = 0; i < values.length - 1; i++) {
        lineBuilder.append(
                nodeWorker.getItemValueFromAttribute(values[i]))
                .append(SEPARATOR);
    }
    lineBuilder
            .append(nodeWorker
                    .getItemValueFromAttribute(values[values.length - 1]));
    return lineBuilder.toString();
}

they're identical except for what I'm passing to the append method from the StringBuilder.

I'm not able to find a way to write the "business logic" of the loop just once.

share|improve this question
add comment

2 Answers

up vote 3 down vote accepted

One way using the Template method pattern:

abstract class LineBuilder {
    protected abstract String getLine(String value);

    public String buildLine(String[] values) {
        StringBuilder lineBuilder = new StringBuilder();
        for (int i = 0; i < values.length - 1; i++) {
            lineBuilder.append(getLine(values[i])).append(SEPARATOR);
        }
        lineBuilder.append(getLine(values[values.length - 1]));
        return lineBuilder.toString();
    }
}

class StringLineBuilder extends LineBuilder {
    protected String getLine(String value) {
        return value;
    }
}

class NodeWorkerLineBuilder extends LineBuilder {
    private final NodeDecorator nodeWorker;

    public NodeWorkerLineBuilder(NodeDecorator nodeWorker) {
        this.nodeWorker = nodeWorker;
    }

    protected String getLine(String value) {
        return nodeWorker.getItemValueFromAttribute(value);
    }
}

Once you define the above classes you can replace the code inside your methods with:

public static String buildLine(String[] values) {
    return new StringLineBuilder().buildLine(values);
}

public static String buildLineFromNodeWorker(String[] values,
        NodeDecorator nodeWorker) {
    return new NodeWorkerLineBuilder(nodeWorker).buildLine(values);
}
share|improve this answer
    
Better explanation of the template method you can find here sourcemaking.com/design_patterns/template_method –  Adam Jul 3 '13 at 5:54
add comment
up vote 3 down vote

A straight forward refactoring:

// Differing bit
private interface Function<X,Y> {
    Y apply(X x);
}

// Common bit
private static String buildLine(String[] values, Function<String, String> f) {
    StringBuilder lineBuilder = new StringBuilder();
    for (int i = 0; i < values.length - 1; i++) {           
        lineBuilder.append(f.apply(s)).append(SEPARATOR);
    }
    lineBuilder.append(values[values.length - 1]);
    return lineBuilder.toString();
}

// call the common routine, supply differing part as a parameter
public static String buildLine(String[] values) {
    return buildLine(values, new Function<String, String>() {
        public String apply(String s) {
            return s;
        }
    });
}

// call the common routine, supply differing part as a parameter
public static String buildLineFromNodeWorker(String[] values, final NodeDecorator nodeWorker) {
    return buildLine(values, new Function<String, String>() {
        public String apply(String s) {
            return nodeWorker.getItemValueFromAttribute(s);
        }
    });
}

Two private methods above can be replaced with the following one if you have Guava libraries:

private static String buildLine(String[] values, Function<String, String> f) {
    return Joiner.on(SEPARATOR).join(
            Iterables.transform(Arrays.asList(values), f));
}
share|improve this answer
    
I cannot use the for loop because I would be writing the last element twice. –  dierre Jul 2 '13 at 6:31
    
@dierre You're right. Removed that suggestion. –  abuzittin gillifirca Jul 2 '13 at 8:23
add comment

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.