Last week I was reviewing some code when I came across something that looked like.
class TypeTranslator<TOrigin, TDestination> {
public TDestination Convert(TOrigin value) {
Type typeOfTo = typeof(TDestination);
TDestination to = (TDestination)typeOfTo.InvokeMember(null, BindingFlags.CreateInstance, null, null, null, CultureInfo.CurrentCulture);
// goes on and copies the contents of each property from the TFrom object instance to the TTo object instance
return to;
}
}
The idea is to translate entity types to data transfer types. The class receives the origin and destination types upon instantiation and when Convert is called it creates a new instance of the destination type and populates its properties from the origin type enumerating through all its properties via reflection.
We all know that reflection is slow when compared to accessing the types and members directly, but the real problem on this code lays on the fact that it tries to instantiate a new object using a parameterless constructor, but there is no guarantee that the class has one.
It has all been working based on the convention assumed on the project that these types will all have a public parameterless constructor. What happens if a new developer comes in and unaware of the convention decides to create a constructor with parameters? The compiler seeing that there’s a constructor will not create the default parameterless constructor anymore. Since there’s nothing else checking it, it will only fail at runtime.
A runtime exception would be caught relatively early if they were using unit testing, but they aren’t using it, so it depends on trusting the developer to do the proper tests.
Well, I’m a developer, but I trust more on the compiler doing that job than on myself or whoever for that matter, so I proposed some changes:
class TypeTranslator<TOrigin, TDestination> where TDestination : new() {
public TDestination Convert2(TOrigin value) {
TDestination to = new TDestination();
// goes on and copies the contents of each property from the TFrom object instance to the TTo object instance
return to;
}
}
This way, the compiler will guarantee that the type used has a parameterless constructor. Finding issues at compile time is much cheaper than at runtime.
