rexcardan / Evil-DICOM

A C# DICOM Library
171 stars 98 forks source link

Avoid dynamic keyword to increase portability and improve compile-time type checking #28

Closed anders9ustafsson closed 9 years ago

anders9ustafsson commented 9 years ago

Hello again Rex,

please find a new pull request primarily focused on further increasing the portability of the Evil-DICOM code.

In the IDICOMElement interface, you have declared DData and DData_ as dynamic properties. If I understand correctly, these properties are mainly provided to enable a non-generic getting and setting functionality for the various element types, right?

Unfortunately, the dynamic keyword prevents my Portable Class Library from targeting .NET 4.0. For reasons unknown to me, the PCL library can only target .NET 4.5 and upwards when dynamic is present. On the other hand, if the dynamic keyword could be avoided, there are no more obstacles for the PCL library to also target .NET 4.0.

The above is my rationale for wanting to get rid of the dynamic keyword. But I also think that for the library as a whole it would be beneficial to remove the use of dynamic. Declaring your property dynamic means that you will be able to apply practically any operation on the property, and the validity will not be checked until run-time. Using static types for DData and DData_ will enable you to capture substantially more potential errors already at compile-time.

My proposal is thus to change the type of DData to object and DData_ to ICollection. Being non-generic types, you still will be able to apply the same getter and setter operations as when they are dynamic, with the additional benefit that further operations on the properties are limited by the API:s of the object and ICollection types, respectively.

I have added a unit test to verify that the same functionality applies to the properties before and after my proposed refactoring.

Or maybe you have specific use cases where dynamic is a must? Please let me know what you think about my proposal.

Best regards, Anders