Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Good strategy to avoid duplicate code

Lets say i have the following scenario:

public class A {
    public String createString(final String value){
        if (value ==  null){
            throw new NullPointerException("value must NOT be null.");
        }
        final StringBuffer sb = new StringBuffer();
        sb.append("A");
        sb.append("B");
        sb.append("C");
        if (value.length() > 3){
            sb.append("D");
            sb.append("E");
        }
        sb.append("Z");
        return sb.toString();   
    }
}

And another class which should do a similar Task:

public class B {
    public String createString(final String value){
        if (value ==  null){
            throw new NullPointerException("value must NOT be null.");
        }
        final StringBuffer sb = new StringBuffer();
        sb.append("A");
        sb.append("B");
        sb.append("C");
        sb.append("Z");
        return sb.toString();   
    }
}

What would be a good strategy to avoid the duplicate code ? What i came up with so far is that the class B which has a subset functionality of A and therefore should extend from the class A and the same tasks should be refactored into protected methods (assuming they are in the same package). This is what it would look like:

public class A {
    public String createString(final String value){
        final StringBuffer sb = createTheFirstPart(value);
        if (value.length() > 3){
            sb.append("D");
            sb.append("E");
        }
        createTheLastPart(sb);
        return sb.toString();   
    }

    protected void createTheLastPart(final StringBuffer sb) {
        sb.append("Z");
    }

    protected StringBuffer createTheFirstPart(final String value) {
        if (value ==  null){
            throw new NullPointerException("value must NOT be null.");
        }
        final StringBuffer sb = new StringBuffer();
        sb.append("A");
        sb.append("B");
        sb.append("C");
        return sb;
    }
}

And the class B:

public class B extends A {
    public String createString(final String value){
        final StringBuffer sb = createTheFirstPart(value);
        createTheLastPart(sb);
        return sb.toString();   
    }
}

Another possible solution would be something like this:

public class A {
    public String createString(final String value){
        if (value ==  null){
            throw new NullPointerException("value must NOT be null.");
        }
        final StringBuffer sb = new StringBuffer();
        sb.append("A");
        sb.append("B");
        sb.append("C");
        addSomeSpecialThings(value, sb);
        sb.append("Z");
        return sb.toString();   
    }

    protected void addSomeSpecialThings(final String value, final StringBuffer sb) {
        if (value.length() > 3){
            sb.append("D");
            sb.append("E");
        }
    }
}

and class B:

public class B extends A {
    public String createString(final String value){
        return super.createString(value);
    }

    protected void addSomeSpecialThings(final String value, final StringBuffer sb) {
        // do nothing
    }
}

Obviously this is not that good because B has an empty impl. of addSomeSpecialThings. Also this excample is a very simple one. For Example there could be more difference within the method so it would not be that easy to extract the same functionality.

My solutions are all about inheritance maybe it also would be better to do this with composition. I also thought that this is probably a canidate for the strategy pattern.

Well whats the best approach to such kind of Problem ? Thanks in advance for any help.

kuku.

like image 509
kukudas Avatar asked Nov 27 '10 10:11

kukudas


2 Answers

I would put the shared code in a superclass of A and B:

public abstract class SomeName {
    public final String createString(final String value){
        if (value ==  null){
            throw new NullPointerException("value must NOT be null.");
        }
        final StringBuffer sb = new StringBuffer();
        sb.append("A");
        sb.append("B");
        sb.append("C");
        addSomeSpecialThings(value, sb);
        sb.append("Z");
        return sb.toString();   
    }

    protected abstract void addSomeSpecialThings(final String value,
            final StringBuffer sb);
}

Then B would look like this:

public class B extends SomeName {
    protected void addSomeSpecialThings(final String value,
            final StringBuffer sb) {}
}

And this would be A:

public class A extends SomeName {
    protected void addSomeSpecialThings(final String value, final StringBuffer sb) {
        if (value.length() > 3){
            sb.append("D");
            sb.append("E");
        }
    }
}
like image 128
thejh Avatar answered Sep 29 '22 09:09

thejh


The described situation is rather simple. I think inheritance is ok here, but i would suggest to create a base class with empty implemented addSomeSpecialThings, and then inherit two classes, A and B, and override the method.

Strategy pattern is appropriate, but not in such simple case. Two situations is too few for an overhead you need to implement the pattern.

like image 21
Vladimir Ivanov Avatar answered Sep 29 '22 10:09

Vladimir Ivanov