remove duplication

Question

I have a class contains 10 methods which are doing almost the same things apart from one key event. Two examples are given below:


Public String ATypeOperation(String pin, String amount){
    doSomething();
    doMoreStuff();
    requestBuilder.buildATypeRequest(pin, amount);
    doAfterStuff();
}



Public String BTypeOperation(String name, String sex, String age){
    doSomething();
    doMoreStuff();
    requestBuilder.buildBTypeRequest(name, sex, age);
    doAfterStuff();
}

As you can see from the above methods, they are similar apart from calling different methods provided by requestBuilder. The rest 8 are similar too. There is a lot duplicated code here. I feel there is a better way to implement this, but don’t know how. Any ideas and suggestions are appreciated.

Thanks, Sarah

@Helper Yes, it is. You can say it is used in doAfterSuff(); — sarahTheButterFly, Aug 03 '10 at 07:23

Bozho · Accepted Answer · 2010-08-03T07:11:53.537

5

Use something like RequestBuilder, that accepts all these kinds of parameters:

public RequestBuilder {
    // setters and getters for all properties

    public Request build() {
         doStuff();
         Request request = new Request(this);
         doAfterStuff();
         return request;
    }
}

and then

new RequestBuilder().setAge(age).setName(name).build();

edited Aug 03 '10 at 07:11

answered Aug 03 '10 at 07:06

Bozho

588,226
146
1,060
1,140

I have a question here. When a client who uses the RequestBuilder(), How do he know what parameters he should set? Take the two methods in my post for example, I would have to ask the client to choose to set 2 of the 5 possible parameters. How can we be sure that the client won't set a wrong parameter? – sarahTheButterFly Aug 04 '10 at 00:41
it sets whichever it wants (has 'at hand'). If he combination is not sufficient, throw an exception. – Bozho Aug 04 '10 at 06:13
You pass the mandatory params in the RequestBuilder constructor and any optional param by additional method calls before calling build. Thats what the builder is made for: many optional params (to reduce telescoping constructors) @bozo you should really set the mandatory params in the builders constructor – atamanroman Aug 04 '10 at 06:16
@fielding of course, but I didn't know there are such - from the above example there is not a single required param. – Bozho Aug 04 '10 at 06:44
I know that you know, but that doesnt come out with the example provided. I think that was what sarah was asking for: "... How do he know what parameters he should set? ..." – atamanroman Aug 04 '10 at 06:59

score 3 · Answer 2 · edited May 23 '17 at 12:27

3

What’s the nearest substitute for a function pointer in Java?

Function Pointers in Java

edited May 23 '17 at 12:27

Community

1
1

answered Aug 03 '10 at 07:03

Mitch Wheat

295,962
43
465
541

extraneon · Answer 3 · 2010-08-03T07:39:04.543

2

interface RequestBuilder {
  void doStuff(params);
}

public RequestBuilder getARequestBuilder() {
  return new RequestBuilder() {
    void doStuff(params) {  
      // impl.details
    }
  }
}    

public RequestBuilder getBRequestBuilder() {
  return new RequestBuilder() {
    void doStuff(params) {  
      // impl.details
    }
  }
}    

public String buildRequest(yourParams, RequestBuilder builder){
  doBefore();
  builder.doStuff(yourParams);
  doAfter();
}

I think this is called the Strategy pattern. It looks a lot like the Command pattern but because you encapsulate an algorithm it seems to be Strategy :)

What Bozho suggest is the Builder pattern.

I recommend you browse through a list of patterns some time, or buy Head First Patterns. Really fun reading.

edited Aug 03 '10 at 07:39

answered Aug 03 '10 at 07:06

extraneon

23,575
2
47
51

I wasnt sure either if this was a command or strategy pattern. Your decoupling an algo (strategy) but on the other hand this is clearly an encapsulated method call (command). Maybe someone can tell why its which pattern. I'd say its command although the method is called immediately :) – atamanroman Aug 04 '10 at 06:10
@fielding I guess it is the intent which matters most. But I only use patterns (sometimes without knowing it's a pattern:) and I am not a theoretical patterns kind of guy. – extraneon Aug 04 '10 at 07:48

score 1 · Answer 4 · answered Aug 03 '10 at 07:29

You could pass the builder object to a generic buildRequest method. Since not only the algorithm but also the arguments vary, i put them into the builder. I dont think thats a nice solution but i wanted to show a command pattern here :D (Extraneon showed how to decouple params and command)

    // call somewhere in the code:
    Builder b = new BTypeBuilder();
    b.age = "20"; b.sex = "female"; b.name = "eve";
    String res = buildRequest(b);

    Public String buildRequest(Builder builder)
    {
        doSomething();
        doMoreStuff();
        builder.build();
        doAfterStuff();
    }

    // Command pattern
    class BTypeBuilder implements Builder
    {
        String name, age, sex;

        // Constructor here

        void build() 
        {
            // Do your stuff here
        }
    }

    class ATypeBuilder implements Builder
    {
        String pin, amount;

        // Constructor here

        void build() 
        {
            // Do your stuff here
        }
    }

    public interface Builder 
    { 
        void build();
    }

I like your solution. But I think it exposes the BTypeBuilder() to a client. Which means the client has to know about all the different types of builders. Isn't that 'tight coupling'? — sarahTheButterFly, Aug 04 '10 at 00:50
You could introduce some sort of Builder-Factory. A static factory method should be sufficient and hides all the different builders. It depends on what your doing. If this is part of your public api, the effort is reasonable. If its not, i would not go so far. Btw: If you like an answer, upvote it ;) — atamanroman, Aug 04 '10 at 06:23
Haha...just upvoted your post! Thanks for answering my question. ;) — sarahTheButterFly, Aug 05 '10 at 03:59

score 0 · Answer 5 · answered Aug 03 '10 at 08:32

In addition to other answers, this might also be useful for you (If you want to just plugin your method, not using your parameters for 'before' and 'after' methods)

interface Function0<R> {
    R apply();
}

public void performOperation(Function0<Void> operation) {
    doSomething(); 
    doBeforeStuff(); 
    operation.apply();
    doAfterStuff(); 

}

then you could use it like this,

    final RequestBuilder builder = new RequestBuilder();
    performOperation(new Function0<Void>() {
        public Void apply() {
            builder.buildATypeRequest("1234", "2445");
            return null;
        }
    });

    performOperation(new Function0<Void>() {
        public Void apply() {
            builder.buildBTypeRequest("1234", "2445", "1234");
            return null;
        }
    });

score 0 · Answer 6 · answered Aug 03 '10 at 09:55

0

Instead of sending a long parameter list just push all the parameters in a map and send that map as argument.

answered Aug 03 '10 at 09:55

rahulmamgain

134
1
6

Hmmm..If parameter list size is less than 6 (or 4?), then it is ok. Actually better than push them in a map I think. How does your user know what to put in the map? – sarahTheButterFly Aug 04 '10 at 00:53
Where does this help anything? You have to add new validation because you cant tell anymore if the map contains fitting objects. It would be better to introduce some sort of Person class, which could group name, sex and age. If your having many params, try to introduce factories or builders. But this has nothing to do with the initial question. – atamanroman Aug 04 '10 at 06:26

remove duplication

6 Answers6