Removing the Undefined unit

[lipchev]

1. Replace with non-nullable value (constructing with BaseUnit by default)
   private readonly MassUnit? _unit;
2. Remove all the guard checks:
   if(unit == MassUnit.Undefined) throw new ArgumentException("The quantity can not be created with an undefined unit.", nameof(unit));

[angularsen]

Fixed in v5

[angularsen]

@lipchev DefaultDataContractJsonSerializerTests is failing on removing the Undefined unit.

Basically, there is now an off-by-1 error in some tests.

However, I realize now that it is a really bad idea to serialize the unit integer value, because units are added alphabetically to the unit enum type, so it is in fact very likely that a unit will get a different integer value over time.

What do you think we should do with this serializer?

[angularsen]

See latest release/v5 branch.

[lipchev]

Yes, that is the limitation of the default behavior- you have to keep your versions in sync. Alternatively, one can create a surrogate (like we did earlier) for the IQuantity or even globally for the Enum type.

Unfortunately, this same limitation applies to the tests. It is a little inconvenient to have to change the expected-string every time a Mass/Information unit is added but at the time it felt better than something like "{\"Value\":1.2,\"Unit\":\"{(int)MassUnit.Milligram}\"}".

[angularsen]

Gotcha. So, I guess we definitely need to provide a surrogate before we start advertising support for the default JSON contract serializer then?

[lipchev]

I don't think so- I would say it works as expected™, given how the Enums are treated by the DataContractJsonSerializer (by design).
Also we cannot provide surrogates for anything other than .net framework.

[angularsen]

Right, but I'm thinking we shouldn't document DataContractJsonSerializer as being supported by Units.NET on the readme/wiki unless we also provide a surrogate that fixes the enum issue. People may then use it without thinking twice, and run into subtle serialization bugs in the future when we add more units.

Currently, only json.net is documented as supported:
https://github.com/angularsen/UnitsNet#serialization

I propose adding a new section covering Json/Xml data contract serializers and describing their current behavior and why the JSON one is not recommended due to this limitation. The XML one serializes enum name, doesn't it?

[lipchev]

The XML one serializes enum name, doesn't it?

Yes it does.

I propose adding a new section covering Json/Xml data contract serializers and describing their current behavior and why the JSON one is not recommended due to this limitation

I agree, although I'm not sure how I would group the serialization sub-sections:

• we could have them split as being `Out-of-the-box' or part of a NuGet
• we could have them as XML, Json, Binary
• they could be (further) split into Recommended and Legacy
• there could be a (simpler) version of the table I showed in Added DataContract annotations to all Quantities {Value, Unit} #907 containing all options

PS It would have been cool to have a vanilla (converter-less) example, that I would have assumed to be compatible with the standard DataContractJsonSerializer, however the last time I checked JsonNet wasn't picking on the [DataContract] but instead relying on the IConvertible interface which- funny enough doesn't have an implementation for string (actually throwing an exception).