Generic toString() for JPA base entity class

Question

In a Java/JPA project I usually define a base class (on which I put the id). The goal is to implement an generic toString() on this base class (named BaseEntity) from which all other entities inherit. We played around with apache commons ReflectionToStringBuilder, but we ran into problems because of cyclic references and the lack of different handling depending on the type of the fields.

My wish is that, if an Entity is printed/logged I get all field names and their values except for fields of type collection, in this case we just want the IDs of these objects to avoid recursion (PS: We can assume that collections on entities always contain other entities and they have an ID)

The desired output pattern is the following

classname[fieldname1=value1, fieldname2=value2,..., collectionname1=[id1,id2,..], collectionname2=[id1,id2,...],...]

a sample output for an entity "Project" which subclasses BaseEntity may look like this:

Project[nameLong=Testproject, nameShort=tpr, support=false, active=true, priority=0, blockers=[20], responsibilities=[13,14,15]]

We came up with the following implementation using reflection:

@Override
public String toString() {
 // init container for field names
 List<String> fieldToPrintout = new ArrayList<String>(0);
 // get all fields from this object (use declared fields to get private
 // and protected fields)
 Field[] fieldsOnTheObject = this.getClass().getDeclaredFields();
 // set the accessibility for field which are private
 AccessibleObject.setAccessible(fieldsOnTheObject, true);
 for (Field field : fieldsOnTheObject) {
 try {
 // ignore static fields
 if (!Modifier.isStatic(field.getModifiers())) {
 // get the object from the field
 Object fieldValue = field.get(this);
 // check if the field is a collection
 if (Collection.class.isAssignableFrom(field.getType())) { 
 // check if the collection generic type is BaseEntity
 if (field.getGenericType() instanceof ParameterizedType) {
 Class<?> genericType = (Class<?>) ((ParameterizedType) field.getGenericType()).getActualTypeArguments()[0];
 if (BaseEntity.class.isAssignableFrom(genericType)) {
 // iterate over all BaseEntities and get all id's
 List<String> ids = ((Collection<BaseEntity>) fieldValue).stream().map(p -> p.getId().toString()).collect(Collectors.toList());
 fieldToPrintout.add(field.getName() + "=[" + String.join(",", ids) + "]");
 }
 }
 } else { // --> no collection
 // note: Field.get(Object) creates wrapper objects for primitive
 fieldToPrintout.add(field.getName() + "=" + fieldValue);
 }
 }
 } catch (IllegalAccessException ex) {
 logger.catching(ex);
 }
 }
 return MessageFormat.format("{0}[{1}]", this.getClass().getSimpleName(), String.join(", ", fieldToPrintout));
}

I'm currently not really happy with the code, I'm sure it can be improved/simplified. Any suggestions?

Answer 1

So, there are a couple of things that can be done with this. First, use methods! Short, well-named methods can make it much easier to read the code. Second, use short-circuiting logic. continue is your friend. Most of your comments don't really contribute much, or can be obviated with good method names. You can also set the size of the ArrayList at creation time, because you know how many fields you're going to be dealing with. In my opinion, something like this looks a lot cleaner:

@Override
public String toString() {
 final Field[] fields = this.getClass().getFields();
 final List<String> fieldsToPrintOut = new ArrayList<String>(fields.length);
 AccessibleObject.setAccessible(fields, true);
 for (final Field field : fields) {
 if (Modifier.isStatic(field.getModifiers())) {
 continue;
 }
 final Object fieldValue;
 try {
 fieldValue = field.get(this);
 } catch (final IllegalAccessException ex) {
 logger.catching(ex);
 continue;
 }
 if (!isCollection(field)) {
 fieldsToPrintOut.add(field.getName() + "=" + fieldValue);
 continue;
 }
 if (!isParameterized(field) || !isBaseEntity(field)) {
 continue;
 }
 final List<String> ids =
 ((Collection<BaseEntity>) fieldValue).stream().map(p -> p.getId().toString()).collect(Collectors.toList());
 fieldsToPrintOut.add(field.getName() + ids);
 }
 return MessageFormat.format(this.getClass().getSimpleName() + fieldsToPrintOut);
}
private static boolean isCollection(final Field field) {
 return Collection.class.isAssignableFrom(field.getType());
}
private static boolean isParameterized(final Field field) {
 return field.getGenericType() instanceof ParameterizedType;
}
private static boolean isBaseEntity(final Field field) {
 final Class<?> genericType =
 (Class<?>) ((ParameterizedType) field.getGenericType()).getActualTypeArguments()[0];
 return BaseEntity.class.isAssignableFrom(genericType);
}

• Antot: Eric, your implementation is definitely more readable than the original, but you have 1) two times <code>if(condition) { continue; }</code> that may be extracted in <code>if (!shouldOutputField(field)) { continue; }</code>. 2) fieldValue initialization would better be extracted into separate method with something returned on exception, otherwise the entire field is ignored in the output. 3) String.join(",", list) is not necessary on Lists: the items are already concatenated with ", " in case of default list.toString() call. 4) there are two typos in deCLaredFields and fieldSToPrintout
• Eric Stein: @Antot 1) I disagree. This style shows why the field should not be output without having to dig into another method. 2) The original method ignores fields with an exception. Adding a method to change the try {} catch(){} into an if (fieldValue == null) is of dubious value, but the OP can certainly do so. 3) Fixed. Good catch. 4) Also fixed. Note that the original method variable was fieldToPrintout, but fieldsToPrintOut is definitely better.
• Antot: Eric, 1) ok; 2) I thought that generally it would be better to see in the output something like "fieldName=[ERROR:not_accessible]" than not to have it at all.
• Eric Stein: @Antot That's up to the OP. I can't tell him how he should handle an error, so I duplicated the functionality.
• Raphael Roth: thanks for the comments, the extracting of methods made it far more readable. I did not cared too much about the excpetion handling so far because I don't see why exceptions should happen at all. But I guess I would be nice to incorporate such things in the returned string

Answer 2

After some time using the solution given by Eric, I discovered certain problems (they are already present in my original solution, therefore I don't want to blame Eric).

• The ID is not given in the output as getDeclaredFields() does NOT include fields defined on the superclass.
• Accessing the private fields directly is not a good idea. It would be better to access the public accessors.
• I'm not sure whether is really a good idea to override toString() in the first place, as toString() may get automatically invoked by frameworks, therefore having a slow toString() is undesireable. I therefore decided to use a separate method for the inspection of the JPA entity.

Here my new solution using org.apache.commons.beanutil:

import java.io.Serializable;
import java.lang.reflect.InvocationTargetException;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import org.apache.commons.beanutils.PropertyUtils;
public abstract class BaseEntity implements Serializable {
 private static final long serialVersionUID = 1326510770497247826L;
 private Long id;
 public Long getId() {
 return id;
 }
 public void setId(Long id) {
 this.id = id;
 }
 public String inspect() {
 return inspect(this);
 }
 private String inspect(Object o) {
 if (o == null) {
 return "null";
 }
 // List to store key/value strings
 List<String> fieldToPrintout = new ArrayList<String>(0);
 // check if object is a collection, if yes, invoke inspect each element
 if (Collection.class.isAssignableFrom(o.getClass())) {
 Collection<Object> col = ((Collection<Object>) o);
 for (Object obj : col) {
 // if element extends BaseEntity, fetch only id
 if (BaseEntity.class.isAssignableFrom(obj.getClass())) {
 fieldToPrintout.add(((BaseEntity) obj).getId() == null ? "null" : ((BaseEntity) obj).getId().toString());
 } else {
 fieldToPrintout.add(obj.toString());
 }
 }
 return MessageFormat.format("{0}[{1}]", o.getClass().getSimpleName(), String.join", ", fieldToPrintout));
 // if the argument is no collection and does not extending BaseEntity
 // --> invoke standard toString()
 } else if (!(BaseEntity.class.isAssignableFrom(o.getClass()))) {
 return o.toString();
 }
 // at this point we know the argument extends BaseEntity, i.e. a is a Bean.
 // map for storing key/values of beans
 Map<String, Object> properties = new HashMap<>();
 // inspect properties:
 try {
 properties = PropertyUtils.describe(o);
 } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
 e.printStackTrace();
 }
 // loop over all properties
 for (Entry<String, Object> entry : properties.entrySet()) {
 if (exclude(entry)) {
 continue;
 }
 fieldToPrintout.add(entry.getKey() + "=" + inspect(entry.getValue()));
 }
 return MessageFormat.format("{0}[{1}]", o.getClass().getSimpleName(), String.join(", ", fieldToPrintout));
 }
 /**
 * helpermethod to exclude unwanted key/value pairs from PropertyUtils.describe(o)
 * 
 * @param entry
 * @return
 */
 private boolean exclude(Entry<String, Object> entry) {
 String key = entry.getKey();
 Object value = entry.getValue();
 if (key.equals("class") && value.getClass().equals(Class.class)) {
 return true;
 }
 return false;
 }
}

A small demo (Company and Employee are standard Beans extending BaseEntity):

Company company = new Company();
company.setId(1L);
company.setName("Super Company");
company.getDomains().add("Engineering");
company.getDomains().add("Consulting");
company.getDomains().add("Training");
company.setSize(100);
Employee employee1 = new Employee();
employee1.setId(1L);
employee1.setActive(true);
employee1.setName("Employee 1");
employee1.setCompany(company);
Employee employee2 = new Employee();
employee2.setActive(false);
employee2.setName("Employee 2");
employee2.setCompany(company);
company.getEmployees().add(employee1);
company.getEmployees().add(employee2);
System.out.println(company.inspect());

Gives:

Company[size=100, name=Super Company, domains=ArrayList[Engineering, Consulting, Training], id=1, employees=HashSet[1, null]]