2
\$\begingroup\$

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.

\$\endgroup\$
3
\$\begingroup\$

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);
}
\$\endgroup\$
3
\$\begingroup\$

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));
}
\$\endgroup\$
  • \$\begingroup\$ I cannot use the for loop because I would be writing the last element twice. \$\endgroup\$ – dierre Jul 2 '13 at 6:31
  • \$\begingroup\$ @dierre You're right. Removed that suggestion. \$\endgroup\$ – abuzittin gillifirca Jul 2 '13 at 8:23

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.