13

I use a third party library. They pass me a POJO that, for our intents and purposes, is probably implemented like this:

public class OurData {
  private String foo;
  private String bar;
  private String baz;
  private String quux;
  // A lot more than this

  // IMPORTANT: NOTE THAT THIS IS A PACKAGE PRIVATE CONSTRUCTOR
  OurData(/* I don't know what they do */) {
    // some stuff
  }

  public String getFoo() {
    return foo;
  }

  // etc.
}

For many reasons, including but not limited to encapsulating their API and facilitating unit testing, I want to wrap their data. But I don't want my core classes to be dependent on their data (again, for testing reasons)! So right now I have something like this:

public class DataTypeOne implements DataInterface {
  private String foo;
  private int bar;
  private double baz;

  public DataTypeOne(String foo, int bar, double baz) {
    this.foo = foo;
    this.bar = bar;
    this.baz = baz;
  }
}

public class DataTypeTwo implements DataInterface {
  private String foo;
  private int bar;
  private double baz;

  public DataTypeOne(String foo, int bar, double baz, String quux) {
    this.foo = foo;
    this.bar = bar;
    this.baz = baz;
    this.quux = quux;
  }
}

And then this:

public class ThirdPartyAdapter {
  public static makeMyData(OurData data) {
    if(data.getQuux() == null) {
      return new DataTypeOne(
        data.getFoo(),
        Integer.parseInt(data.getBar()),
        Double.parseDouble(data.getBaz()),
      );
    } else {
      return new DataTypeTwo(
        data.getFoo(),
        Integer.parseInt(data.getBar()),
        Double.parseDouble(data.getBaz()),
        data.getQuux();
      );
  }
}

This adapter class is coupled with the other few classes that MUST know about the third party API, limiting it's pervasiveness through the rest of my system. However... this solution is GROSS! In Clean Code, page 40:

More than three arguments (polyadic) requires very special justification--and then shouldn't be used anyway.

Things I've considered:

  • Creating a factory object rather than a static helper method
    • Doesn't solve the problem of having a bajillion arguments
  • Creating a subclass of DataTypeOne and DataTypeTwo that has a dependent constructor
    • Still has a polyadic protected constructor
  • Create entirely separate implementations that conform to the same interface
  • Multiple of the above ideas simultaneously

How should this situation be handled?


Note this is not an anti-corruption layer situation. There's nothing wrong with their API. The problems are:

  • I don't want MY data structures to have import com.third.party.library.SomeDataStructure;
  • I can't construct their data structures in my test cases
  • My current solution results in very very high argument counts. I want to keep argument counts low, WITHOUT passing in their data structures.
  • That question is "what is a anti corruption layer?". My question is "how can I use a pattern, any pattern, to solve this scenario?"

I'm not asking for code, either (otherwise this question would be on SO), just asking for enough of an answer to enable me to write the code effectively (which that question does not provide).

  • If there are several such 3rd party POJOs, it may be worth the effort to write custom test code that uses a Map with some conventions (e.g. name the keys int_bar) as your test input. Or use JSON or XML with some custom intermediary code. In effect, sort of a DSL for testing com.thirdparty. – user949300 Jan 29 '15 at 18:13
  • The full quote from Clean Code: The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification — and then shouldn’t be used anyway. – Lilienthal Jan 29 '15 at 22:23
  • 11
    Blind adherence to a pattern or programming guideline is its own anti-pattern. – Lilienthal Jan 29 '15 at 22:31
  • 2
    "encapsulating their API and facilitating unit testing" Sounds like this might be a case of over-testing and/or Test-induced Design Damage to me (or indicative that you could design this differently to begin with). Ask yourself this: does this really make your code easier to understand and change and reuse? I'd put my money on "no." How realistically likely is it that you'll ever swap out this library? Probably not very. If you do swap it out, does this really make it any easier to drop a completely different one in place? Again, I'd bet on "no." – jpmc26 Jan 30 '15 at 1:35
  • 1
    @JamesAnderson I just reproduced the full quote because I found it interesting but it wasn't clear to me from the snippet whether it referred to functions in general or constructors specifically. I didn't mean to endorse the claim and, as jpmc26 said, my next comment should give you some indication that I wasn't doing so. I'm not sure why you feel the need to attack academics but using polysyllables doesn't make someone an academic elitist perched on his ivory tower above the clouds. – Lilienthal Jan 30 '15 at 10:08
10

The strategy I've used when there are several initialization parameters is to create a type which just contains the parameters for initialization

public class DataTypeTwoParameters {
    public String foo;  // use getters/setters instead if it's appropriate
    public int bar;
    public double baz;
    public String quuz;
}

Then the constructor for DataTypeTwo takes a DataTypeTwoParameters object, and DataTypeTwo is constructed via:

DataTypeTwoParameters p = new DataTypeTwoParameters();
p.foo = "Hello";
p.bar = 4;
p.baz = 3;
p.quuz = "World";

DataTypeTwo dtt = new DataTypeTwo(p);

This gives a lot of opportunity to make it clear what all the parameters going into DataTypeTwo are and what they mean. You can also provide sensible defaults in the DataTypeTwoParameters constructor so that only values which need to be set can be done so in any order the consumer of the API likes.

  • Interesting approach. Where would you put a relevant Integer.parseInt? In a setter, or outside the parameters class? – durron597 Jan 29 '15 at 17:44
  • 5
    Outside the parameters class. The parameters class should be a "dumb" object, and shouldn't try to do anything other than express what the required inputs and their types are. Parsing should be done elsewhere, like: p.bar = Integer.parseInt("4"). – Erik Jan 29 '15 at 17:51
  • 7
    this sounds like a Parameter Object pattern – gnat Jan 29 '15 at 17:52
  • 9
    ...or anti-pattern. – Telastyn Jan 29 '15 at 18:54
  • 1
    ... or you could just rename DataTypeTwoParameters to DataTypeTwo. – immibis Jan 30 '15 at 0:18
14

You really have two separate concerns here: wrapping an API and keeping the argument count low.

When wrapping an API, the idea is to design the interface as if from scratch, knowing nothing other than the requirements. You say there is nothing wrong with their API, then in the same breath list a number of things wrong with their API: testability, constructability, too many parameters in one object, etc. Write the API you wish you had. If that requires multiple objects instead of the one, do that. If it requires wrapping one level higher, to the objects that create the POJO, do that.

Then once you have your desired API, the parameter count may no longer be an issue. If it is, there are a number of common patterns to consider:

  • A parameter object, as in Erik's answer.
  • The builder pattern, where you create a separate builder object, then call a number of setters to set the parameters individually, then create your end object.
  • The prototype pattern, where you clone subclasses of your desired object with the fields already set internally.
  • A factory, which you are already familiar with.
  • Some combination of the above.

Note that these creational patterns often end up calling a polyadic constructor, which you should consider okay when it's encapsulated. The problem with polyadic constructors isn't calling them once, it's when you're forced to call them every single time you need to construct an object.

Note that usually it's much easier and more maintainable to pass through to the underlying API by storing a reference to the OurData object and forwarding the method calls, rather than trying to reimplement its internals. For example:

public class DataTypeTwo implements DataInterface {
  private OurData data;

  public DataTypeOne(OurData data) {
    this.data = data;
  }

   public String getFoo() {
    return data.getFoo();
  }

  public int getBar() {
    return Integer.parseInt(data.getBar());
  }
  ...
}
  • First half of this answer: great, very helpful, +1. Second half of this answer: "pass through to the underlying API by storing a reference to the OurData Object" - this is what I'm trying to avoid, at least in the base class, to ensure there's no dependency. – durron597 Jan 29 '15 at 18:35
  • 1
    That's why you only do it in one of your implementations of DataInterface. You create another implementation for your mock objects. – Karl Bielefeldt Jan 29 '15 at 18:37
  • @durron597: yes, but you already know how to solve that problem if it really bothers you. – Doc Brown Jan 29 '15 at 18:37
1

I think you might be interpreting Uncle Bob's recommendation too strictly. For normal classes, with logic and methods and constructors and such, a polyadic constructor does indeed feel a lot like code smell. But for something that is strictly a data container that exposes fields, and is generated by what is essentially a Factory object already, I don't think it's too bad.

You can use the Parameter Object pattern, as suggested in a comment, can wrap these constructor parameters for you, what your local data type wrapper is is already, essentially, a Parameter object. All your Parameter object will be doing is packing the parameters up (How will you create it? With a polyadic constructor?) and then unpacking them a second later into an object which is almost identical.

If you don't want to expose setters for your fields and call them, I think that sticking to a polyadic constructor inside a well-defined and encapsulated factory is fine.

  • The problem is, the number of fields in my data structure has changed multiple times and probably will change again. Which means I need to refactor the constructor in all my test cases. The parameter pattern with sensible defaults sounds like a better way to go; having a mutable version that gets saved into the immutable form could make my life easier in many ways. – durron597 Jan 29 '15 at 18:01

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.