views:

127

answers:

2

So I did some refactoring and two of my classes now look exactly the same except for their constructors.

The classes wrap an API object that isn't very pretty, and add some functionality that belong at the edge of the API.

class A extends API {
  public A {
    this.APIOption = Option1;
    this.AnotherAPIOption = Option2;
    // a few more
  }

  public ArrayList<String> somethingTheAPIDoesntDo() {
    // Do something the API doesn't provide but I use alot
  }
  // Other shared methods
}

class B extends API {
  public B {
    this.APIOption = Option3;
    this.AnotherAPIOption = Option4;
    // a few more
  }

  public ArrayList<String> somethingTheAPIDoesntDo() {
    // Do something the API doesn't provide but I use alot
  }
  // Other shared methods
}

Does it make sense to push the common code between the two to an Abstract Base class, and have the subclasses only implement their constructors with the specialized option settings? It makes sense on paper, but something feels weird/counterintuitive about it. Am I missing a pattern here?

Possible DRYer Solution

class A extends BetterAPIBase {
  public A {
    this.APIOption = Option1;
    this.AnotherAPIOption = Option2;
    // a few more
  }
}

class B extends BetterAPIBase {
  public B {
    this.APIOption = Option3;
    this.AnotherAPIOption = Option4;
    // a few more
  }
}    

abstract class BetterAPIBase extends API {
  public Better APIBase() {}
  public ArrayList<String> somethingTheAPIDoesntDo() {
    // Do something the API doesn't provide but I use alot
  }
  // Other methods
}

EDIT

Static Factory Pattern is nice, but I think I may also add an Interface that includes the common methods I added.

I would make the BetterAPI class also implement IBetterAPI, which would only expose the methods I added wherever I declare the instance's type as IBetterAPI.

interface IBetterAPI{
      public ArrayList<String> somethingTheAPIDoesntDo();
      // other methods I added in BetterAPI
}

//somewhere else:
IBetterAPI niceApi = BetterAPI.createWithOptionSetA();
niceApi.somethingTheAPIDoesntDo();

// Can't do this, nice and hidden.
niceApi.somethingBaseAPIDoes(string uglyOptions, bool adNauseum); 
+12  A: 

Wouldn't it be simpler to have one class with a parametrized (possibly private) constructor and multiple static factory methods?

class BetterAPI extends API {
  private BetterAPI(APIOption option, AnotherAPIOption anotherOption) {
    this.APIOption = option;
    this.AnotherAPIOption = anotherOption;
  }

  public static BetterAPI createWithOptionSetA() {
    return new BetterAPI(Option1, Option2);
  }

  public static BetterAPI createWithOptionSetB() {
    return new BetterAPI(Option3, Option4);
  }

  // ...
}

The core of your problem seems to be that you can't have multiple parameterless constructors in the same class. Static factory methods offer a nice solution to this.

Péter Török
Good answer, I mentioned it as a comment, but was too lazy to write out the code!
Chad
+1  A: 

That's pretty close to what I'd do. Though if the "few more" makes for a lot of redundant code in the constructor, I'd write:

abstract class BetterAPIBase extends API { 
  public BetterAPIBase(String APIOption, int AnotherAPIOption, ... more ...) {
    this.APIOption=APIOption;
    this.AnotherAPIOption=AnotherAPIOption;
    // a few more
  } 
  public ArrayList<String> somethingTheAPIDoesntDo() { 
    // Do something the API doesn't provide but I use alot 
  } 
  // Other methods 
} 

class A extends BetterAPIBase { 
  public A { 
    super(Option1, Option2, ... more ...);
  } 
} 

class B extends BetterAPIBase { 
  public B { 
    super(Option3, Option4, ... more ...);
  } 
}     

Of course if any of the option values are the same then they wouldn't need to be passed, which is where real redundancy elimination would come.

Jay