2

I have a Java factory method with a varargs array of Objects at the end. The array can contain any combination of Strings and ScaledJpegs. The theory being that an HTML table cell can contain any number of text nodes or <image> nodes and it will line-wrap the images as though they were just funny-shaped words.

// constructor
public Cell(TextStyle ts, float w, CellStyle cs, Object... r) {
    if (w < 0) {
        throw new IllegalArgumentException("A cell cannot have a negative width");
    }
    if (r != null) {
        for (Object o : r) {
            if ( (o != null) &&
                 !(o instanceof String) &&
                 !(o instanceof ScaledJpeg) ) {
                throw new IllegalArgumentException(INVALID_ROW_TYPE_STR);
            }
        }
    }
    textStyle = ts; width = w; cellStyle = cs; rows = r;
    avgCharsForWidth = (int) ((width * 1220) / textStyle.avgCharWidth());
}

In Scala, I might use a list of Either[String,ScaledJpeg]. But some day I'll probably make this method allow something else, say another nested Cell, but doing so would break existing code that only expected two possible types. At least the way it is, future changes won't break existing code.

The current solution works, allows for expansion, and is relatively easy to use, except that it defeats type safety and can only throw an exception at runtime if someone passes something unexpected as a vararg. For instance, I just forgot to add toString() to a StringBuilder and it blew up at runtime. So already that's a third class to account for (Since toString() is defined on every object, I don't want to call it on whatever is passed because doing so for most object types would be an error much further down in the code violating the fail-fast behavior of this example).

java.lang.String can't implement any new interfaces and that's probably a good thing. Even if I could make a wrapper class, everything about the way text and images are treated by this class is completely different except that they are line-wrapped together the way they would be in HTML (or in a Word doc).

As I write this, it occurs to me that I should probably use a Builder pattern, make a CellBuilder class that has addText() and addImage() methods that does away with the varargs constructor. This would encapsulate the underlying List in a way that the user of the API gets 100% compile-time type safety. But out of curiosity, I'm still posting this in case there are other creative and possibly better solutions out there.

  • When you need arbitrary arguments of specific types, a builder pattern is the way to go as you mentioned. – user22815 Aug 12 '14 at 2:03
2

What you want is a Row interface with two implementations: one for images and one for strings. Then your Cell constructor takes a Row... r. This lets the compiler verify your types, and keeps you from having if image... else if string... all over the place. Adding a new type is simple, just create another implementation. If you need a new method in the interface, your compiler ensures all your implementations have it.

  • You mean like a wrapper class, only an interface that can hold either a String or a ScaledJpeg thus: new Cell(ts, w, cs, Wrapper.of("hello"), Wrapper.of(myScaledJpg), Wrapper.of("world")) – GlenPeterson Aug 11 '14 at 20:16
  • No, not just a wrapper. You have other places in your Cell class that do things like if (rows[0] instanceof String) renderString((String)rows[0]); else if (rows[0]) instanceof ScaledJpeg) renderJpeg((ScaledJpeg)rows[0]);. Instead you want a StringRow class and a JpegRow class that each implement a render() method of the Row interface. – Karl Bielefeldt Aug 11 '14 at 20:37
  • Making a hierarchy of types as you and @Daenyth suggest might be the best general solution to this kind of problem. But doing so makes the API user think they could pass anything that implements that interface to the Cell constructor. That's just not possible in this situation due to line-wrapping strings and passing them and images separately to an underlying 3rd party API: github.com/GlenKPeterson/PdfLayoutManager/blob/master/src/main/… I think the Builder pattern is most appropriate here. Thank you both for your help! – GlenPeterson Aug 11 '14 at 21:33
  • I take it back, you may be right... I'll think more, but don't waste your time on this. – GlenPeterson Aug 11 '14 at 21:52
  • 1
    I did as you suggested and created a Renderable interface. It required major refactoring to apply in this particular example, but is a very good solution. There are still some things I'd like hidden that are exposed and vice-versa, but until I can articulate them, it's good enough. – GlenPeterson Aug 21 '14 at 15:08
2

Since you assign rows = r and r: Array[Object], I'd use this approach:

sealed trait HasRow {
  def row: Object
}

class StringRow(s: String) extends HasRow {
  override def row = s // or some logic
}

class JpegRow(j: ScaledJpeg) extends HasRow {
  override def row = j // or some logic
}

class Cell(ts: TextStyle, w: Float, cs: CellStyle, r: HasRow*) {
 // ...
}

As you mention, a builder style approach is also possible and might be better.

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.