Hold and validate EAN code - follow-up

Question

Based on suggestions and some thoughts I made some refactorings of my classes to hold EAN13 codes, I created a contract interface which now is extended by BarCode classes, since there will be other types of BarCodes, like DUN-14, UPC-A...

public interface BarCode {}

I decide to move all the validations to factory class, so the EAN13 class looked like this:

@Embeddable
public class Ean13 implements BarCode {
 @Column(name = "ean_code", nullable = true, length = 13)
 private String code;
 public Ean13() {
 }
 public Ean13(String code) {
 this.code = code;
 }
 @Override
 public String toString() {
 return code;
 }
}

I changed the pre-generate exception creating a custom InvalidBarCodeException exception.

In the first moment, this was created as a RuntimeException. But I think this is the case where checked is much better suited, because I'm forced to deal with invalid codes.

public class InvalidBarCodeException extends Exception {
 private String code;
 private static final String INVALID_EAN = "INVALID EAN CODE";
 public InvalidBarCodeException(String code) {
 super(INVALID_EAN + " "+code);
 this.code = code;
 }
 public String getCode() {
 return code;
 }
}

The validation was moved to a Predicate in a separated class:

public class BarCodePredicate {
 public static Predicate<String> isValidEan13() {
 return p -> isValid(p);
 }
 private static boolean isValid(String code) {
 if (code == null || code.length() != 13) {
 return false;
 }
 if (!CharMatcher.DIGIT.matchesAllOf(code)) {
 return false;
 }
 String codeWithoutVd = code.substring(0, 12);
 int pretendVd = Integer.valueOf(code.substring(12, 13));
 String[] evenOdd = SplitterByIndex.split(codeWithoutVd, idx -> idx % 2 == 0);
 int evenSum = sumStringDigits(evenOdd[0]);
 int oddSum = sumStringDigits(evenOdd[1]);
 int oddFator = oddSum * 3;
 int sumResult = oddFator + evenSum;
 int dv = getEanVd(sumResult);
 if (pretendVd != dv) {
 return false;
 }
 return true;
 }
 private static int sumStringDigits(String s) {
 return s.chars().map(n ->
 Character.getNumericValue(n)
 ).sum();
 }
 private static int getEanVd(int s) {
 return 10 - (s % 10);
 }
}

So I use it in a factory class:

public interface BarCodeFactory {
 BarCode create(String code) throws InvalidBarCodeException;
 default boolean isValid(String code, Predicate<String> predicate) {
 return predicate.test(code);
 }
}
public class Ean13Factory implements BarCodeFactory { 
 @Override
 public BarCode create(String code) throws InvalidBarCodeException {
 if (!isValid(code, BarCodePredicate.isValidEan13())){
 throw new InvalidBarCodeException(code);
 }
 return new Ean13(code);
 }
}

In the Product class now is simply a set method (it will be changed to BarCode interface):

public class Product{
 public void setEan(Ean13 ean) {
 this.ean = ean;
 }
}

And invalid codes are treated outside:

Product p = new Product();
p.setDescription(name);
p.setUrl(url);
try {
 BarCode ean = new Ean13Factory().create(code);
 //TODO: refactoring.
 p.setEan((Ean13) ean);
} catch (InvalidBarCodeException e) {
 logInvalidCode(e, code);
}

Does anyone have other suggestions?

Answer 1

interface BarCode

The interface is a good idea, but it shouldn't be just a marker interface. It seems very probable that all the implementations that you will add later, will contain a String code value. So, a method declaration like String getCode(); would be useful here. It will allow to access the value when necessary, without worrying about the concrete BarCode type behind.

Over-engineering

But I'm also asking myself if a separate class per BarCode type is really necessary here. Should ean just remain a basic field within Product?

@Entity
public class Product { ...
 @Column
 private String eanCode;
 ...
}

The idea would be to validate the candidate eanCode value before setting it on the object. Not sure that dedicated classes are necessary, because they are just wrappers for the only field, aren't they?

Exceptions Handling

Taking into account the current listing, the code

try {
 BarCode ean = new Ean13Factory().create(code);
 p.setEan((Ean13) ean);
} catch (InvalidBarCodeException e) {
 logInvalidCode(e, code);
}

does exactly the same thing as if it were

if (BarCodePredicate.isValid(code) {
 p.setEan(new Ean13(code));
} else {
 logInvalidCode(code);
}

This resembles drastically to what we can find in item#59, page 247 of Effective Java.

• alexpfx: You are right, the original intention was that Ean know how to validate itself. But what about the fact that I have more than one type of bar code. encapsulate it in a class does not make sense in this case? About the exception: now I inderstood: if the method can't throw more than one exception type I should use a simple if.
• Antot: 1) If you intend to create a field per each code type in Product, I think that they should remain Strings. If you want to avoid such pollution, indeed there should be separate types per code, but their JPA mapping should also be different (e.g. dedicated @Entity with [id_product, code_type, code] values). 2) It's wrong about exceptions. Their number does not matter, just one may be nice sometimes. Exceptions should be used for exceptional situations, but here you have a pre-condition of barcode value validity, which can help make the decision in the flow.