How to write a unit test to check copy constructor is in sync with class properties?

Question

Recently we had a bug in our system that is caused by forgetting to assign a newly added class property in the copy constructor.

For example:

public class MyClass {

    private Long companyId;
    private Long classId;
    private Double value;   <=  newly added

    public MyClass(MyClass myClass) {
        this.setCompanyId(myClass.getCompanyId());
        this.setClassId(myClass.getClassId());
        this.setValue(myClass.getValue());   <= we forget this line
    }

I want to write a unit test that guarantees to catch the missing copy assignment when a new property is added. How should I proceed with this?

Seng

by making your fields final you force your constructor to set all of the fields – Enphytotic

@RoyShahaf: What about the case in which that's not tenable? There's no guarantee that this is constructed with a builder which would handle that or there could be the possibility that some fields may not be set. This isn't about guaranteeing that there is a value, it's guaranteeing that the values are the same across instances. null is a perfectly acceptable value in that scenario. – Zoospore

@RoyShahaf: I don't want to enforce final, just for the sake of copy constructor. We could easily forget to make the field final too leaving us with the same problem. – Seng

@Zoospore it's true that for some cases is a perfectly acceptable value. How ever it's been a while since using null was considered good practice if you're able to avoid doing so (if such a time ever existed). – Enphytotic

@RoyShahaf: null is the default field value when it's unassigned... – Zoospore

@Seng if you have a good reason (and it really needs to be good) to use mutable types (and seemingly we are talking about mutable data types, or copy constructors and serializations make less sense) go ahead and use reflection to test that. Otherwise I would recommend using an immutable approach. You can use lombok annotations (or AutoValue annotations) to help guarantee that fields are final. – Enphytotic

@Zoospore as Doug Lea supposedly said: "null sucks" and as Tony Hoare said: "I call it my billion-dollar mistake" – Enphytotic

@RoyShahaf If all fields are final, then a copy constructor is likely not necessary (unless it does a deep copy of other objects that are mutable). – Moise

@MarkRotteveel that's true, for data types that are not deeply immutable a copy constructor makes sense when you want to make a deep copy that protects you on both sides – Enphytotic

I would first ask why you even have a copy constructor. Strictly speaking there isn't such a thing in Java. I haven't used one in 26 years. Why do you think you need to copy instances of this class? – Thamos

Answer 1

IMO, there are three ways to solve this problem

1. use a copy-constructor that will be auto-updated (using Lombok for example)

@Builder(toBuilder=true)
class Foo {
   int x;
   ...
}
Foo f0 = Foo.builder().build();
Foo f1 = f0.toBuilder().build();  // copy

2. Create a tests that populate an object with non-default values, and then use reflection/Jackson to read all its key-value pairs, and make sure that the value is not a default value (null, 0, 0.0, false, '\0')

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
// other imports

public class FooTest {
  @Test
  public void testCopyConstructor_shouldCopyAllFields() {
    ObjectMapper mapper = new ObjectMapper();
    // only use field, don't use getters
    mapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE);
    mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);

    // should be instantiated with non-default values
    Foo original = new Foo();
    original.setSimilarity(0.1);
    original.setStatus(SomeEnum.PENDING);
    // ...

    Foo copied = new Foo(original);


    Map<String, Object> originalMap = mapper.convertValue(original, new TypeReference<HashMap<String, Object>>(){});
    for (Map.Entry<String, Object> entry : originalMap.entrySet()) {
      // if we add new fields and forget to update this test, this test should fail
      assertTrue(isNotDefaultValue(entry.getValue()), "Forgot to update this test: " + entry.getKey());
    }

    Map<String, Object> resultingMap = mapper.convertValue(copied, new TypeReference<HashMap<String, Object>>(){});
    for (Map.Entry<String, Object> entry : resultingMap.entrySet()) {
      Object expectedValue = originalMap.get(entry.getKey());
      assertEquals(entry.getValue(), expectedValue);
    }
  }

  private static boolean isNotDefaultValue(Object value){
    if (value == null)
      return false;
    boolean defaultBoolean = false;
    char defaultChar = '\0';
    byte defaultByte = 0;
    short defaultShort = 0;
    int defaultInt = 0;
    long defaultLong = 0;
    float defaultFloat = 0;
    double defaultDouble = 0;
    List<Object> defaultValues = List.of(defaultBoolean, defaultChar, defaultByte, defaultShort,
      defaultInt, defaultLong, defaultFloat, defaultDouble);
    return !defaultValues.contains(value);
  }
}

3. Use a complex object-randomizer, such as EasyRandom, and use it in your unit tests and do your copy constructor

import java.util.Objects;
// other imports

public class FooTest {
  private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

  @Test
  public void testCopyConstructor_shouldCopyAllFields() {
    EasyRandom generator = new EasyRandom();
    
    for (int i = 0; i < 4; i++){  // you can do it multiple times if you're unsure
      Person person = generator.nextObject(Person.class);
      Person copied = new Person(person);

      // or if you afraid of forgetting to update your .equals() method, 
      // just convert both of them to hashmaps (using jackson) 
      // and compare the hashmaps
      assertEquals(copied, person);  
    }
  }
}

Scherle

Very interesting EasyRandom class. – Vanesavanessa

Answer 2

I'd do this with reflection.

• New up an instance of MyClass. (A)
• Through reflection, assign a value to every field. This can be accomplished with getClass().getDeclaredFields().
• New up a blank instance of MyClass. (B)
• Run the copy constructor on A against B.
• Through reflection, determine if the fields in A are equal to B.

Zoospore

At step 2, assigning a value to every field could be a little tricky because in the real scenario we have complex classes as a class property as well, what do you think? – Seng

If they're part of your copy constructor, you have no choice. You have to guarantee that every field matches, no matter how complex. – Zoospore

Can you provide example code with a complex object, I am not sure how to do it with reflection – Seng

I'd expect that a copy constructor doesn't clone field values, so comparison by identity should work just fine. – Girdle

Answer 3

1) To add to Makoto post, I once used a method to control programmatically the correctness of a cloning. But it's something hard and long.

I give you the code I've used as an unit test. It's surely perfectible:

/**
 * Vérification du bon clonage d'un objet.
 * @param o Objet original.
 * @param clone Objet qui se dit clone du premier.
 * @param champsExclus Champs qu'il faut exclure de la vérification d'assertNotSame().
 */
public void controlerClonage(Object o, Object clone, String... champsExclus) {
   assertNotNull(o, MessageFormat.format("L''objet de classe {0} dont le clonage doit être contrôlé ne peut pas valoir null.", o.getClass().getName()));
   assertNotNull(clone, MessageFormat.format("L''objet de classe {0} dont le clonage doit être contrôlé ne peut pas valoir null.", o.getClass().getName()));

   // Cloner l'objet puis vérifier qu'il est égal à son original.
   assertEquals(o, clone, MessageFormat.format("L''objet cloné {0} et son original devraient être égaux.", o.getClass().getSimpleName()));
   assertNotSame(o, clone, MessageFormat.format("L''objet cloné {0} et son original ne devraient pas partager le même pointeur.", o.getClass().getSimpleName()));

   // Enumérer toutes les variables membres de l'objet.
   Field[] champs = o.getClass().getDeclaredFields();

   for(Field champ : champs) {
      // Si le champ est parmi la liste de ceux à exclure, ne pas en tenir compte.
      if (isChampExclu(champ, champsExclus)) {
         continue;
      }

      // Si l'objet est un type primitif ou un String, l'assertNotSame() ne pourra pas s'appliquer.
      // En revanche, l'assertEquals(), lui, toujours.
      champ.setAccessible(true);

      // Lecture de la valeur originale.
      Object valeurOriginal = null;

      try {
         valeurOriginal = champ.get(o);
      }
      catch(IllegalArgumentException | IllegalAccessException e) {
         fail(MessageFormat.format("L''obtention de la valeur du champ {0} dans l''objet original {1} a échoué : {2}.", champ.getName(),
               o.getClass().getSimpleName(), e.getMessage()));
      }

      // Lecture de la valeur clonée.
      Object valeurClone = null;

      try {
         valeurClone = champ.get(clone);
      }
      catch(IllegalArgumentException | IllegalAccessException e) {
         fail(MessageFormat.format("L''obtention de la valeur du champ {0} dans l''objet cloné {1} a échoué : {2}.", champ.getName(),
               clone.getClass().getSimpleName(), e.getMessage()));
      }

      assertEquals(valeurOriginal, valeurClone, MessageFormat.format("La valeur de la variable membre {0} de l''objet {1} et de son clone devrait être égales.", champ.getName(), clone.getClass().getSimpleName()));

      // Les types primitifs, les chaînes de caractères et les énumérations, ne voient pas leurs pointeurs vérifiés.
      // Et cela n'a de sens que si les valeurs de ces pointeurs sont non nuls.
      if (valeurOriginal != null && valeurClone != null) {
         if (champ.getType().isPrimitive() == false && champ.getType().equals(String.class) == false
               && Enum.class.isAssignableFrom(champ.getType()) == false)
            assertNotSame(valeurOriginal, valeurClone, MessageFormat.format("La variable membre {0} de l''objet {1} et de son clone ne devraient pas partager le même pointeur.", champ.getName(), clone.getClass().getSimpleName()));
      }
   }
}

/**
 * Déterminer si un champ fait partie d'une liste de champs exclus.
 * @param champ Champ.
 * @param champsExclus Liste de champs exclus.
 * @return true, si c'est le cas.
 */
private boolean isChampExclu(Field champ, String[] champsExclus) {
   for (String exclus : champsExclus) {
      if (exclus.equals(champ.getName()))
         return true;
   }

   return false;
}

/**
 * Alimenter un objet avec des valeurs par défaut.
 * @param objet Objet.
 * @return Objet lui-même (alimenté).
 */
public Object alimenter(Object objet) {
   double nDouble = 1.57818;
   int nInt = 1;
   long nLong = 10000L;
   
   Class<?> classe = objet.getClass();
   
   while(classe.equals(Object.class) == false && classe.getName().startsWith("java.") == false) {
      for(Field field : classe.getDeclaredFields()) {
         // Ignorer certains types
         if (field.getType().equals(Class.class) || field.getType().equals(ArrayList.class)
            || field.getType().equals(List.class)|| field.getType().equals(Set.class)
            || field.getType().equals(HashSet.class) || field.getType().equals(HashMap.class)) {
            continue;
         }
         
         // Ecarter les champs statiques.
         if (Modifier.isStatic(field.getModifiers())) {
            continue;
         }
         
         // Champs de type texte.
         if (field.getType().equals(String.class)) {
            setValue(field, objet, "*" + field.getName() + "*");
            continue;
         }
         
         // Champs de type LocalDate.
         if (field.getType().equals(LocalDate.class)) {
            setValue(field, objet, LocalDate.now());
            continue;
         }
         
         // Champs de type LocalDateTime.
         if (field.getType().equals(LocalDateTime.class)) {
            setValue(field, objet, LocalDateTime.now());
            continue;
         }
         
         // Champs de type URL.
         if (field.getType().equals(URL.class)) {
            try {
               URL url = new URL("http://a.b.c/test");
               setValue(field, objet, url);
            }
            catch(MalformedURLException e) {
               throw new RuntimeException("Mauvaise préparation d'URL pour test toString : " + e.getMessage());
            }
            
            continue;
         }
         
         // Champs de type ZonedDateTime.
         if (field.getType().equals(ZonedDateTime.class)) {
            setValue(field, objet, ZonedDateTime.now());
            continue;
         }
         
         // Champs de type texte.
         if (field.getType().equals(Double.class) || field.getType().equals(Double.TYPE)) {
            setValue(field, objet, nDouble ++);
            continue;
         }
         
         // Champs de type integer.
         if (field.getType().equals(Integer.class) || field.getType().equals(Integer.TYPE)) {
            setValue(field, objet, nInt ++);
            continue;
         }
         
         // Champs de type long.
         if (field.getType().equals(Long.class) || field.getType().equals(Long.TYPE)) {
            setValue(field, objet, nLong ++);
            continue;
         }
         
         // Champs de type énumération
         if (Enum.class.isAssignableFrom(field.getType())) {
            @SuppressWarnings("unchecked") 
            Class<Enum<?>> enumeration = (Class<Enum<?>>)field.getType();
            Enum<?>[] constantes = enumeration.getEnumConstants();
            setValue(field, objet, constantes[0]);
            
            // On en profite pour vérifier toutes ses constantes.
            for(Enum<?> constante : constantes) {
               System.out.println(MessageFormat.format("{0}.{1}.{2} = {3}", classe.getName(), field.getName(), constante.name(), constante.toString()));
            }
            
            continue;
         }
         
         if (field.getType().getName().startsWith("java.") == false) {
            try {
               field.setAccessible(true);
               Object membre = field.get(objet);
               
               // Ecarter les champs statiques et abstraits.
               if ((Modifier.isStatic(field.getModifiers()) && Modifier.isAbstract(field.getModifiers())) == false) {
                  // Si l'objet n'est pas initialisé, tenter de le faire.
                  if (membre == null) {
                     try {
                        membre = field.getType().getDeclaredConstructor().newInstance();
                     }
                     catch(@SuppressWarnings("unused") InstantiationException | InvocationTargetException | NoSuchMethodException | SecurityException e) {
                        // On laisse passer, on ne pourra  pas l'attribuer.
                     }
                  }
                  
                  // Si l'on a obtenu le membre ou si on l'a créé, on l'alimente et on l'affecte.
                  if (membre != null && membre != objet) {
                     alimenter(membre);
                     setValue(field, objet, membre);
                  }
               }
               
               continue;
            }
            catch(IllegalArgumentException | IllegalAccessException e) {
               System.err.println(MessageFormat.format("{0}.{1}.{2} n'a pas pu être assigné : {3}", objet.getClass().getName(), field.getName(), e.getMessage()));
            }
         }

         // Indiquer les champs que l'on a pas pu assigner.
         System.err.println(MessageFormat.format("non assigné : {0}.{1}", field.getName(), field.getType().getName()));
      }
      
      classe = classe.getSuperclass();
   }
   
   return objet;
}

/**
 * Fixer une valeur à un champ.
 * @param field Champ dans l'objet cible.
 * @param objet Objet cible.
 * @param valeur Valeur à attribuer.
 */
private void setValue(Field field, Object objet, Object valeur) {
   field.setAccessible(true);
   
   try {
      field.set(objet, valeur);
   }
   catch(IllegalArgumentException | IllegalAccessException e) {
      System.err.println(MessageFormat.format("{0}.{1} n'a pas pu être assigné : {2}", objet.getClass().getName(), field.getName(), e.getMessage()));
   }
}

---

2) This isn't about automatic control by U.T., but for develop time, inside the IDE:

These days, I removed all the clone() methods I had, having learnt that they are a bad practice, to replace them with copy constructors.

I noticed that if an IDE like IntelliJ detects in a class MyClass a constructor of the form MyClass(MyClass source) {...} it will warn if one assignation is missing.

The class name is surrounded in yellow, with the message: Copy constructor does not copy field 'population'.

Vanesavanessa