I’m working with a model that currently looks like this:
class Sign {
enum Type { A, B }
Color color;
String textLine1;
String textLine2;
Type type;
Sign(Type type, Color color, String textLine1, String textLine2) { ... }
}
A sign of type A has a color and 2 lines of text. A sign of type B has a color and only 1 line of text. Therefore they’re instantiated like so:
Sign signA = new Sign(A, someColor, "line1", "line2");
Sign signB = new Sign(B, someColor, "line1", null);
When working with these instances, code checks for the type property and reads the appropriate fields.
I think this approach may be flawed as a B sign should not expose properties it doesn’t hold (line2). It’s also unsightly to be passing null to a bunch of properties it doesn’t care about at instantiation time.
I’m wondering if it would be appropriate to use inheritance instead, where a base class would contain the shared fields and the type.
abstract class Sign {
enum Type { A, B }
Color color;
String textLine1;
Type type;
Sign(Type type, Color color, String textLine1) { ... }
}
class SignA extends Sign {
String textLine2;
SignA(Color color, String textLine1, String textLine2) {
super(A, color, textLine1);
this.textLine2 = textLine2;
}
}
class SignB extends Sign {
SignB(Color color, String textLine1) {
super(B, color, textLine1);
}
}
Is the above a recommended pattern for modeling models of this type? Note that these models have no behavior and are purely bags of properties.
2
Inheritance is the right way to model this domain, however enumeration is not necessary here. You already have distinctive characteristic of each sign – the type, which can be checked via instanceof or via reflection.
Polymorphism example:
abstract class Sign {
abstract void printType();
}
class SignA extends Sign {
void printType() {
System.out.println("Sign A");
}
}
class SignB extends Sign {
void printType() {
System.out.println("Sign B");
}
}
...
Sign sign = ...
sign.printType();
instanceof example:
Sign s = ...
if (s instanceof SignA) { ... }
else if (s instanceof SignB) { ... }
else {
// safety check for future code modifications, in which you add new type
// but forget to add a handler
throw new UnsupportedOperationException("Sign type "
+ s.getClass().getName()
+ " not supported");
}
Reflection example (Java 8):
Map<Class<? extends Sign>, Consumer<Sign>> processors = ...;
...
processors.put(SignA.class, s -> System.out.println("Sign A"));
processors.put(SignB.class, s -> System.out.println("Sign B"));
...
Sign sign = ...;
Consumer<Sign> processor = processors.get(sign.getClass());
processor.accept(sign);
If these are only “bags of properties”, I’d go with the simplest solution:
class SignB {
Color color;
String line1;
SignB(Color color, String line1) { ... }
}
class SignA extends SignB {
String line2;
SignA(Color color, String line1, String line2) {
super(color, line1);
this.line2 = line2;
}
A less restrictive approach (you can’t enforce 1 or 2 lines) but more generic (you always iterate over the available lines):
class Sign {
Color color;
List<String> lines;
Sign(Color color, String... lines) { ... }
}
Sign signA = new Sign(Color.RED, "hello");
Sign signB = new Sign(Color.BLUE, "hello", "world");
A third approach. However I never used such an approach and I don’t know how viable it is:
class Sign {
Color color;
String line1;
Optional<String> line2;
Sign(Color color, String line1) {
this(color, line1, null);
}
Sign(Color color, String line1, String line2) {
this.color = color;
this.line1 = line1;
this.line2 = Optional.ofNullable(line2);
}
}