I have a data structure with data units containing different types of data. I’ve wrapped the data in “Field” objects so that each field is able to independently parse user input in a desired way.
public abstract class<T> AField {
protected T value = null;
public T getValue() {
return this.value;
}
// This is the main reason I'm using a custom Field class
public abstract void parse(String data) throws ParseException;
}
public class IntegerField extends AField<Integer>{
public void parse(String data) {
super.value = data.trim();
}
}
public class StringField extends AField<String> {
...
}
// Note this has the same type as StringField but different parse implementation
public class PhoneNumberField extends AField<String> {
....
}
public class DataUnit {
private AField[] fields;
private IntegerField number = new IntegerField();
private StringField name = new StringField();
public DataUnit() {
this.fields = new Field {
this.number,
this.name
}
}
public void parseAll(String... data) throws ParseException {
for (int i = 0; i < this.fields.length; i++) {
fields[i].parse(data[i]);
}
}
// Now the troublesome methods
public Integer getNumber() {
return this.number.getValue(),
}
public void parseNumber(String data) throws ParseException {
this.number.parse(data);
}
public String getName() {
...
}
public void parseName(String data) throws ParseException {
...
}
}
You can see how the number of methods would quickly go up as I add more types of fields to the DataUnit class or more methods to the AField class. DataUnit has to pass all the methods of all the Field classes forward because I’d rather not expose the Field objects directly to other classes. What’s the best way around this method mess? Are there better patterns to achieve this?
3
I designed and implemented a hierarchy like this many years ago (before generics) so here are some comments. They may help stop you from repeating my mistakes 🙂
The proliferation of methods like parseNumber(), parseName() suggests that the class hierarchy needs adjusment, yet your hierarchy looks well designed.
My approach would be a combination of techniques suggested by Robert Harvey and mdolanci.
- Create an abstract class that has a
parse(String data) throws ParseExceptionmethod. Your AField class looks to do this now, although I would reconsider the class name (I suggest BaseField). - Create overloads for IntegerField() StringField() etc, with suitable parse logic for the type. Again this is exactly what you are doing.
- Your DataUnit class looks to be a container for an arbitrary list of fields, but has a couple of dedicated fields (number and name) that I don’t quite understand. If every DataUnit must have a number and name, then they are correct. But they won’t be duplicated should you add more concrete AField types like PhoneNumberField or ZipCodeField. For instance calling asZipCode() on a list of fields is incorrect, as the list might contain multiple ZipCodeField instances. You can of course add a
findFirstZipCodeField()method that iterates down the field list usinginstanceofto find the first field of that type.
I also suggest you move the actual validation out into static methods in a separate class focussed on string validation (as suggested by @mdolanci), preferably packaged in a separate validation library that knows nothing about fields. The day will come when you want to reuse this logic in a REST interface, Android app or something you have not thought of yet. Not making this separation was one of my biggest design mistakes, and has led to unnecessary code duplication in our applications.
Just create helper class with static helper methods.
Example:
public class DataHelper {
public static String asString(final Object value) {
return asString(value, null);
}
public static String asString(final Object value, final String defaultValue) {
if (value instanceof String)
return (String) value;
return defaultValue;
}
public static Number asNumber(final Object value) {
return asNumber(value, null);
}
public static Number asNumber(final Object value, final Number defaultValue) {
if (value instanceof Number)
return (Number) value;
// If necessary you can parse string values
if (value instanceof String && ((String) value).matches("^[0-9]\.?[0-9]*$"))
return Double.valueOf((String) value);
return defaultValue;
}
public static Integer asInteger(final Object value) {
return asInteger(value, null);
}
public static Integer asInteger(final Object value, final Integer defaultValue) {
Number rv = asNumber(value);
if (rv != null)
return rv.intValue();
return defaultValue;
}
public static Double asDouble(final Object value) {
return asDouble(value, null);
}
public static Double asDouble(final Object value, final Double defaultValue) {
Number rv = asNumber(value);
if (rv != null)
return rv.doubleValue();
return defaultValue;
}
public static String asPhoneNumber(final Object value) {
return asPhoneNumber(value, null);
}
// Here you could return custom object instead of string (if needed)
public static String asPhoneNumber(final Object value, final String defaultValue) {
String rv = asString(value);
if (rv != null) {
// Do your transformations
// ...
return rv;
}
return defaultValue;
}
public static Boolean asBoolean(final Object value) {
return asBoolean(value, null);
}
public static Boolean asBoolean(final Object value, final Boolean defaultValue) {
if (value instanceof Boolean)
return (Boolean) value;
if (value instanceof Number)
return ((Number) value).intValue() > 0;
if (value instanceof String && ((String) value).toLowerCase().matches("true|false|1|0|yes|no"))
return ((String) value).toLowerCase().matches("true|1|yes");
return defaultValue;
}
// ... etc.
}
Usage:
public class Usage {
public static void main(String[] args) {
final Map<String, Object> sampleData = ImmutableMap.of(
"name", "Sample name",
"active", "1",
"age", 37,
"weight", "75.7"
);
final String name = DataHelper.asString(sampleData.get("name"));
final Boolean active = DataHelper.asBoolean(sampleData.get("active"), false);
final Integer age = DataHelper.asInteger(sampleData.get("age"), 0);
final Double weight = DataHelper.asDouble(sampleData.get("weight"));
// ... etc.
}
}
1