search

deislabs / cnab-netstandard

.NET Standard 2.0 Client Library for CNAB
https://cnab.io
MIT License
12 stars 5 forks source link

Re-visit the parameter abstraction hierarchy and encapsulation #16

Open radu-matei opened 5 years ago

radu-matei commented 5 years ago

As defined in #11, there might be room for improvement of the parameter class hierarchy, as well as what is publicly exposed by the Cnab.Bundle namespace.

itowlson commented 5 years ago

The Parameter<T> things of the previous iteration aren't really serialisation objects anyway. For serialisation you should have a Plain Old Data type that maps naively to the JSON schema. If it needs to be public you stuff it in a Serialisation or Json namespace. Then in your Parse or Load method you deserialise into that and then transform it into the application-friendly API. Don't try to overload the serialisation objects to be the consumer API. I've made this mistake too many times!

itowlson commented 5 years ago

You have a typo in the issue title which I don't have permission to edit and CANNOT UNSEE.

itowlson commented 5 years ago

Another idea to consider: instead of surfacing MinValue, MaxLength, etc. on the relevant subclasses, have the base class/interface expose a Constraints collection (which may be empty). This allows UIs to see if there are constraints and surface them for user guidance, without requiring casting to a derived class.