7

I wrote a simple class to demonstrate a chain style method design:

public class Cal {

    private Cal(){}

    private boolean isCheckArguments = false;

    public static Cal useAbs() {
        return new Cal(){   
          @Override int check(int i) {
                return Math.abs(i);
            }};
    }

    public static Cal useNormal() {
        return new Cal();
    }   

    public Cal checkArguments() {
        isCheckArguments =true;
        return this;
    }

     int check(int i){ return i;}

     public int plus(int i, int j) {    
        if(isCheckArguments && i<j){
            throw new IllegalArgumentException("i<j!");
        }
        return check(i+j);
    }
}

So the client code can be:

Cal cal = Cal.useAbs().checkArguments();
int sum = cal.plus(100,2);//IllegalArgumentException occurs

Cal cal2 = Cal.useAbs();
int sum2 = cal.plus(-100,2);//98

Cal cal3 = Cal.useNormal();
int sum3 = cal.plus(-100,2);//-98

My question is: Is it a reasonable design? comparing to: int plus(int a, int b, boolean useAbs, boolean checkArguments). Thanks!

卢声远 Shengyuan Lu
  • 31,208
  • 22
  • 85
  • 130

4 Answers4

5

Sounds like you want a fluent interface to build a service class. Guava does similar things. You'd do something like this:

public interface Cal {
   int plus(int a, int b);
}

public class CalBuilder {
    class InternalCal implements Cal {
       boolean useAbs;
       boolean checkArgs;
       public int plus(int a, int b) {
          if(checkArgs) {
             // blah, blah blah
          }
          if(useAbs) {
             // doodle bug, doodle darn
          }
          return a+b; // whatevs
       }
    }
    boolean absSet=false;
    InternalCal holder=new InternalCal();
    public CalBuilder useNormal() {
        if(absSet) { throw new IllegalArgumentException(); } // already called
        holder.useAbs=false;    
        absSet=true;
        return this;
    }

    public CalBuilder useAbs() {
        if(absSet) { throw new IllegalArgumentException(); } // already called
        holder.useAbs=false;    
        absSet=true;
        return this;
    }

    public CalBuilder checkArguments() {
       if(holder.checkArgs) { throw new IllegalArgumentException(); }
       holder.checkArgs=true;
       return this;
    }

    public Cal build() {
       return holder;
    }
}

Usage would look like this:

Cal cal=new CalBuilder().useAbs().checkArguments().build();
int sum=cal.plus(1,2);
Joseph Ottinger
  • 4,911
  • 1
  • 22
  • 23
  • Pretty sure that `CalBuilder` should be a `static` class, then it would be called like this: `Cal cal = new Cal.Builder()...` – Matt Ball Apr 25 '11 at 15:59
  • You could do it a lot of ways, sure. You wouldn't normally want a builder to be static, however, because then you wouldn't be able to call it from multiple threads and you'd have things colliding with each others' state. – Joseph Ottinger Apr 25 '11 at 16:05
  • 1
    I think you misunderstand me. The builder objects wouldn't be static, the `Builder` _class_ would be. See [Effective Java, Item 2](http://books.google.com/books?id=ka2VUBqHiWkC&lpg=PP1&dq=effective%20java&pg=PA11#v=onepage&q&f=false) (and/or [this answer](http://stackoverflow.com/questions/5007355#5007435)) – Matt Ball Apr 25 '11 at 16:26
  • Yeah, I misunderstood what you were saying. :) – Joseph Ottinger Apr 25 '11 at 16:30
3

This is called a fluent interface, and looks pretty reasonable to me.

One thing I might suggest changing is the name of the class: Calc is more obviously a calculator than Cal (which could be a calendar instead of a calculator).

Matt Ball
  • 354,903
  • 100
  • 647
  • 710
  • 1
    `Calculator` would be even better. Full words are generally easier to read. If you want to name an instance of it `calc`, that's fine, but don't name the type that. – ColinD Apr 25 '11 at 16:01
2

The best design is a simple design. What you are doing is obfuscating the code. It's more difficult to read and fix errors than the "plus(a, b, useAbs, checkArguments)". Also, in useNormal you are returning the same object than "new Cal ()" because you are overrididing the check(int) method and returning the parent's implementation with super.check(int)

Gabriel Llamas
  • 18,244
  • 26
  • 87
  • 112
1

IMHO, this "chaining" approach is not that commonly used in Java, except in the case of Builders or things that are essentially builders.

What you're describing here is actually a calculator builder.

You should probably have a CalculatorBuilder class, which you instantiate, set multiple things (e.g., useAbs, checkArguments, etc.), and eventually call "build". Build will return a calculator that knows nothing about how it was built except for the state it was initialized with.

Also, I don't personally like a design that mixes builder-style logic (e.g., "useAbs"), and things that affect the state of the underlying object (such as checkArguments). I'd say pick one. Either generate a default calculator and set everything later, or have a builder that sets everything and then creates instances whose functionality and behavior cannot be changed.

Uri
  • 88,451
  • 51
  • 221
  • 321
  • The builder pattern is usually used to construct immutable objects and/or replace constructors with many (possibly optional) arguments. I don't see either of those here. – Matt Ball Apr 25 '11 at 15:30
  • I disagree - at least from his example it seems like each calculator operates in a completely different way, there is no benefit for letting a calculator change its state – Uri Apr 25 '11 at 16:16