Robust organization data class (partial followup)

Question

This question is a partial followup from my previous question, however the requirements have changed: I now need to be able to store multiple data entries for some fields.

I also have added two hooks into the OrganizationBuilder, one that validates the given input, and one that formats the given input, mainly used to achieve consistency.

And lastly, the requirement that all fields must be populated, has been dropped.

public enum OrganizationBuilderMode {
    THROW_EXCEPTION_ON_VALIDATION_FAILURE,
    IGNORE_ON_VALIDATION_FAILURE;
}

---

public interface OrganizationField { }

---

public enum SingleOrganizationField implements OrganizationField {
    ADDRESS,
    BRANCH_CODE,
    CHAMBER_OF_COMMERCE,
    CITY,
    ORGANIZATION_ID,
    ORGANIZATION_NAME,
    POSTAL_ADDRESS,
    POSTAL_CITY,
    POSTAL_ZIP_CODE,
    VAT_NUMBER,
    ZIP_CODE
}

---

public enum MultiOrganizationField implements OrganizationField {
    ACCOUNT_NUMBER,
    IBAN_NUMBER,
}

---

public class Organization implements Serializable {
    private static final long serialVersionUID = 28484399283838343L;

    private final EnumMap<MultiOrganizationField, List<String>> multiFieldMap = new EnumMap<>(MultiOrganizationField.class);
    private final EnumMap<SingleOrganizationField, String> singleFieldMap = new EnumMap<>(SingleOrganizationField.class);

    Organization(final EnumMap<MultiOrganizationField, List<String>> multiFieldMap, final EnumMap<SingleOrganizationField, String> singleFieldMap) {
        this.multiFieldMap.putAll(multiFieldMap);
        this.singleFieldMap.putAll(singleFieldMap);
    }

    public List<String> getAccountNumbers() {
        return Collections.unmodifiableList(multiFieldMap.get(ACCOUNT_NUMBER));
    }

    public Optional<String> getAddress() {
        return Optional.ofNullable(singleFieldMap.get(ADDRESS));
    }

    public Optional<String> getBranchCode() {
        return Optional.ofNullable(singleFieldMap.get(BRANCH_CODE));
    }

    public Optional<String> getChamberOfCommerce() {
        return Optional.ofNullable(singleFieldMap.get(CHAMBER_OF_COMMERCE));
    }

    public Optional<String> getCity() {
        return Optional.ofNullable(singleFieldMap.get(CITY));
    }

    public List<String> getIbanNumbers() {
        return Collections.unmodifiableList(multiFieldMap.get(IBAN_NUMBER));
    }

    public Optional<String> getOrganizationId() {
        return Optional.ofNullable(singleFieldMap.get(ORGANIZATION_ID));
    }

    public Optional<String> getOrganizationName() {
        return Optional.ofNullable(singleFieldMap.get(ORGANIZATION_NAME));
    }

    public Optional<String> getPostalAddress() {
        return Optional.ofNullable(singleFieldMap.get(POSTAL_ADDRESS));
    }

    public Optional<String> getPostalCity() {
        return Optional.ofNullable(singleFieldMap.get(POSTAL_CITY));
    }

    public Optional<String> getPostalZipCode() {
        return Optional.ofNullable(singleFieldMap.get(POSTAL_ZIP_CODE));
    }

    public Optional<String> getVatNumber() {
        return Optional.ofNullable(singleFieldMap.get(VAT_NUMBER));
    }

    public Optional<String> getZipCode() {
        return Optional.ofNullable(singleFieldMap.get(ZIP_CODE));
    }

    private Object writeReplace() {
        return new SerializationProxy(this);
    }

    private void readObject(final ObjectInputStream stream) throws InvalidObjectException {
        throw new InvalidObjectException("Proxy required");
    }

    private static class SerializationProxy implements Serializable {
        private static final long serialVersionUID = 484894399233454L;

        private final EnumMap<MultiOrganizationField, List<String>> multiFieldMap;
        private final EnumMap<SingleOrganizationField, String> singleFieldMap;

        private SerializationProxy(final Organization organization) {
            this.multiFieldMap = organization.multiFieldMap;
            this.singleFieldMap = organization.singleFieldMap;
        }

        private Object readResolve() {
            return new Organization(multiFieldMap, singleFieldMap);
        }
    }
}

---

public class OrganizationBuilder {
    private final static Map<OrganizationField, Predicate<String>> DEFAULT_VALIDATION_MAPPING = new HashMap<>();
    static {
        DEFAULT_VALIDATION_MAPPING.put(IBAN_NUMBER, IbanNumberValidator::validate);
        DEFAULT_VALIDATION_MAPPING.put(VAT_NUMBER, VatNumberValidator::validate);
    }

    private final static Map<OrganizationField, UnaryOperator<String>> DEFAULT_FORMATTER_MAPPING = new HashMap<>();
    static {
        DEFAULT_FORMATTER_MAPPING.put(IBAN_NUMBER, String::toUpperCase);
        DEFAULT_FORMATTER_MAPPING.put(VAT_NUMBER, String::toUpperCase);
    }

    private final OrganizationBuilderMode organizationBuilderMode;
    private final Map<OrganizationField, Predicate<String>> validationMapping = new HashMap<>();
    private final Map<OrganizationField, UnaryOperator<String>> formatterMapping = new HashMap<>();

    private final EnumMap<MultiOrganizationField, List<String>> multiFieldMap = new EnumMap<>(MultiOrganizationField.class);
    {
        Arrays.stream(MultiOrganizationField.values())
                .forEach(field -> multiFieldMap.put(field, new ArrayList<>()));
    }
    private final EnumMap<SingleOrganizationField, String> singleFieldMap = new EnumMap<>(SingleOrganizationField.class);

    public static OrganizationBuilder newDefaultBuilder(final OrganizationBuilderMode organizationBuilderMode) {
        Objects.requireNonNull(organizationBuilderMode, "organizationBuilderMode");
        return new OrganizationBuilder(organizationBuilderMode, DEFAULT_VALIDATION_MAPPING, DEFAULT_FORMATTER_MAPPING);
    }

    public static OrganizationBuilder newWithMappings(
            final OrganizationBuilderMode organizationBuilderMode,
            final Map<OrganizationField, Predicate<String>> validationMapping, 
            final Map<OrganizationField, UnaryOperator<String>> formatterMapping
    ) {
        return new OrganizationBuilder(
                Objects.requireNonNull(organizationBuilderMode, "organizationBuilderMode"),
                Objects.requireNonNull(validationMapping, "validationMapping"), 
                Objects.requireNonNull(formatterMapping, "formatterMapping")
        );
    }

    private OrganizationBuilder(
            final OrganizationBuilderMode organizationBuilderMode, 
            final Map<OrganizationField, Predicate<String>> validationMapping, 
            final Map<OrganizationField, UnaryOperator<String>> formatterMapping
    ) {
        this.organizationBuilderMode = Objects.requireNonNull(organizationBuilderMode, "organizationBuilderMode");
        this.validationMapping.putAll(Objects.requireNonNull(validationMapping, "validationMapping"));
        this.formatterMapping.putAll(Objects.requireNonNull(formatterMapping, "formatterMapping"));
    }

    public OrganizationBuilder accountNumber(final String accountNumber) {
        return putMultiField(ACCOUNT_NUMBER, accountNumber, "accountNumber");
    }

    public OrganizationBuilder address(final String address) {
        return putSingleField(ADDRESS, address, "address");
    }

    public OrganizationBuilder branchCode(final String branchCode) {
        return putSingleField(BRANCH_CODE, branchCode, "branchCode");
    }

    public OrganizationBuilder chamberOfCommerce(final String chamberOfCommerce) {
        return putSingleField(CHAMBER_OF_COMMERCE, chamberOfCommerce, "chamberOfCommerce");
    }

    public OrganizationBuilder city(final String city) {
        return putSingleField(CITY, city, "city");
    }

    public OrganizationBuilder ibanNumber(final String ibanNumber) {
        return putMultiField(IBAN_NUMBER, ibanNumber, "ibanNumber");
    }

    public OrganizationBuilder organizationId(final String organizationId) {
        return putSingleField(ORGANIZATION_ID, organizationId, "organizationId");
    }

    public OrganizationBuilder organizationName(final String organizationName) {
        return putSingleField(ORGANIZATION_NAME, organizationName, "organizationName");
    }

    public OrganizationBuilder postalAddress(final String postalAddress) {
        return putSingleField(POSTAL_ADDRESS, postalAddress, "postalAddress");
    }

    public OrganizationBuilder postalCity(final String postalCity) {
        return putSingleField(POSTAL_CITY, postalCity, "postalCity");
    }

    public OrganizationBuilder postalZipCode(final String postalZipCode) {
        return putSingleField(POSTAL_ZIP_CODE, postalZipCode, "postalZipCode");
    }

    public OrganizationBuilder vatNumber(final String vatNumber) {
        return putSingleField(VAT_NUMBER, vatNumber, "vatNumber");
    }

    public OrganizationBuilder zipCode(final String zipCode) {
        return putSingleField(ZIP_CODE, zipCode, "zipCode");
    }

    private OrganizationBuilder putMultiField(final MultiOrganizationField multiOrganizationField, final String fieldValue, final String fieldName) {
        Objects.requireNonNull(multiOrganizationField, "multiOrganizationField");
        Objects.requireNonNull(fieldName, "fieldName");
        Objects.requireNonNull(fieldValue, "fieldValue:" + fieldName);
        if (!validationMapping.getOrDefault(multiOrganizationField, field -> true).test(fieldValue)) {
            if (organizationBuilderMode == THROW_EXCEPTION_ON_VALIDATION_FAILURE) {
                throw new IllegalArgumentException("Field " + fieldName + " cannot be validated: " + fieldValue);
            }
        }
        multiFieldMap.get(multiOrganizationField).add(
                formatterMapping.getOrDefault(multiOrganizationField, field -> field).apply(fieldValue));
        return this;
    }

    private OrganizationBuilder putSingleField(final SingleOrganizationField singleOrganizationField, final String fieldValue, final String fieldName) {
        Objects.requireNonNull(singleOrganizationField, "singleOrganizationField");
        Objects.requireNonNull(fieldName, "fieldName");
        Objects.requireNonNull(fieldValue, "fieldValue:" + fieldName);
        if (!validationMapping.getOrDefault(singleOrganizationField, field -> true).test(fieldValue)) {
            if (organizationBuilderMode == THROW_EXCEPTION_ON_VALIDATION_FAILURE) {
                throw new IllegalArgumentException("Field " + fieldName + " cannot be validated: " + fieldValue);
            }
        }
        singleFieldMap.put(singleOrganizationField, 
                formatterMapping.getOrDefault(singleOrganizationField, field -> field).apply(fieldValue));
        return this;
    }

    public Organization build() {
        return new Organization(multiFieldMap, singleFieldMap);
    }
}

Answer 1

There are a number of things here to comment on. First up, you don't ask for anything in particular to be reviewed. So, I just scanned the code, looking for 'oddities'.

There are some aspects which concern me.

Marker interfaces are seldom a good idea. Why do you have OrganizationField... I cannot see a single place where you actually use it as the marker. All actual usages are defined as either SingleOrganizationField or MultiOrganizationField. It adds no value that I can see.

Additionally, you have a complciated SerializationProxy mechanism set up. I cannot see a reason to have it though. What's wrong with just plain serialization? If there is a reason, I can't see it, so it should probably be commented. Serialization proxies are an ugly workaround to an overly complicated process. Don't make it more complicated unless you really need them.

By all accounts, the whole purpose of these classes are to provide a way for a class to encapsulate a lot of data fields, and then allow those to be serialized. The data fields are stored in a Map. Classes of this type are unfortunately bulky, and ugly. Unfortunately, though, your solution is still bulky, and a different sort of ugly.

I would stick with the more traditional format of having a whole bunch of setter methods, and instance fields. Keep it only a standard form of ugly, not an indirected/abstracted ugly.

Finally, the use of Optionals is.... unconventional in this format. Perhaps things will change more with Java 8, but I can't see a reason why the methods have to return an optional, when a null will suffice. You are creating a lot of typically unnecessary overhead.