-
Notifications
You must be signed in to change notification settings - Fork 404
-
I think we've largely covered the generics route for doing this in #714.
But have you considered boxing the Quantity.Value with something like the QuantityValue that is used in the constructor? It seems to me that we'd only need to implement IConvertible, add a few operators like QuantityValue + QuantityValue, QuantityValue + double, QuantityValue + decimal and possibly some constructor supplied strategy for determining the resulting type of a mixed type operation.
Example usage:
QuantityValue value = Mass.FromGrams(1m).Milligrams; // contains decimal
double actualValue = value; // that's doing Decimal.ToDouble
decimal mayOverflow = (decimal)value; // option A: explicit casting required (may overflow with default strategy)
decimal maxDecimal = (decimal) new QuantityValue(double.MaxValue, TypeStrategy.HandleOverflows)) // option A with custom strategy for handling overflows by using Decimal.Min/Max
decimal possibleInformationLoss = value; // option B (ambiguous when using var): safe implicit casting- assuming HandleOverflows is always true
Mass resul1 = Mass.FromGrams(1m) + Mass.FromGrams(1m); // contains decimal
Mass result2 = Mass.FromGrams(1m) + Mass.FromGrams(1d); // contains decimal (default strategy -> first parameter type)
Mass result3 = Mass.FromGrams(1d) + Mass.FromGrams(1m, TypeStrategy.Preserve); // contains decimal (default strategy + decimal strategy = decimal strategy)
Ratio positiveInfinity = Mass.FromGrams(1d, TypeStrategy.HandleOverflows) / Mass.FromGrams(0m, TypeStrategy.Preserve); // contains QuantityValue.PositiveInfinity which converts to Double.PositiveInfinity or Decimal.MassValue
Ratio divisionByZeroException = Mass.FromGrams(1d) / Mass.FromGrams(0d); // throws DivisionByZero (currently throws ArgumentException)
I'm not yet sure what the TypeStrategy (or whatever the name might be) would have as options but something along the lines of:
- Auto: If both operands are Auto- the final type determined by the order of parameters
- Preserve: If the strategy for the first operand is Auto, the type of the second operand is used
- HandleOverflows (this could either be an added flag, or maybe a second Enum)- in practice I don't think anybody that would care enough to use the decimals would end up mixing them in operations with doubles, and it seems even less likely that an actual conversion overflow would ever occur- but, having the possibility of avoiding the DivisionByZeroException (by using the dedicated QuantityValue.NaN/Infinity values) is something that I feel could be quite useful (here is my typical use case).. I would even set it globally in my AppInit if I could..
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 9 comments 35 replies
-
Clever idea, although it does feel a bit complicated at first glance.
My initial thinking is that we might be better off letting the user choose a numeric type across the board, similar to Math.NET matrices, but I do see the benefit of allowing any numeric type and giving the option to control overflows/infinity.
I'll let the idea mature a bit in my head.
Beta Was this translation helpful? Give feedback.
All reactions
-
CC @tmilnthorp
Beta Was this translation helpful? Give feedback.
All reactions
-
I had thought of something similar, albeit constructing with an IConvertible. Of course it's not CLS compliant, so that went out the window.
I don't think this is a bad strategy. Does it have any pros/cons over switching to generics at some point?
Beta Was this translation helpful? Give feedback.
All reactions
-
This was also independently raised here: #1048
As for using QuantityValue as the internal value, maybe. I haven't really thought about it, but maybe there is some merit to it. I'm trying to come up with counter-arguments not to do so, and I think the best I can think of right now is that serialization will be more complicated and it probably adds some extra bytes for memory sensitive applications.
The best argument I could think of against might be serialization, but perhaps if we stick with double/decimal and not Complex as discussed there, then maybe serialization is unaffected?
I think QuantityValue instead of double may solve most of the gripes we have with double vs decimal.
I don't see generics happening anytime soon. I think it's a huge breaking change to go from value types to reference types and also I think value types is the better choice.
We could keep double / decimal as-is internally, but expose it differently via the interface, so you get QuantityValue from IQuantity.Value and decimal or double from concrete implementations like Information and Length:
Information oneByte1 = Information.FromBytes(1); IQuantity oneByte2 = (IQuantity)oneByte1; var bytes1 = oneByte1.Value; // decimal var bytes2 = oneByte2.Value; // QuantityValue
Beta Was this translation helpful? Give feedback.
All reactions
-
double doesn't implicitly cast to decimal (nor the other way round).
One can implicitly cast double to QuantityValue, but also not the other way round (which is good, as it would cause ambiguities IMHO). In this case, we would need to make sure we're not loosing precision when casting decimal to QuantityValue.
So this would definitely be a breaking change and would need to go to V5.
Beta Was this translation helpful? Give feedback.
All reactions
-
You are right, implict casts only go into QuantityValue, while explicit cast is required for getting values out. If we were to use this type as the return value, I would add DoubleValue and DecimalValue properties wrapping the explicit cast, to make it more discoverable.
Beta Was this translation helpful? Give feedback.
All reactions
-
Sounds reasonable, even though it would require another indirection to get the actual data:
Console.WriteLine($"It's {myVar.Kilometers.DecimalValue} kilometers to go");
Since V5 has the printing improved, that should partially account for this ;-)
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
To be clear, I only intended for IQuantity.Value to return QuantityValue, and instead let Information.Value return decimal directly.
So myVar.Kilometers would be double in this case assuming it was statically known as type Length, while myIQuantityVar.Value would require invoking DecimalValue or DoubleValue.
We also have IDecimalQuantity to make this even more fun, where it could in fact return decimal also.
Beta Was this translation helpful? Give feedback.
All reactions
-
This is getting confusing... I guess we need some kind of prototype to see how this all works out.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1 -
😄 1
-
Generics don’t require reference types btw
Beta Was this translation helpful? Give feedback.
All reactions
-
Ah, I keep mixing that one up. This is like the 3rd time. Do you recall the summary of previous generics discussions? What are pros/cons of generics vs QuantityValue?
Beta Was this translation helpful? Give feedback.
All reactions
-
At first glance, I would really like to avoid having to change existing code from Length.FromMeters(1) to Length<double>.FromMeters(1). I guess we could work around that by letting Length extend Length<double>, and similarly Information extend Information<decimal>.
Beta Was this translation helpful? Give feedback.
All reactions
-
IQuantityValue.Value could still return QuantityValue, as discussed above. This might be a good compromise of the two solutions.
Beta Was this translation helpful? Give feedback.
All reactions
-
I guess we could work around that by letting Length extend Length, and similarly Information extend Information.
If only structs allowed derivation 😞
And C# doesn't have a type alias where we could say typedef Length = Length<double>
Beta Was this translation helpful? Give feedback.
All reactions
-
One (big) problem was how to make them work as single nuget, or alternatively how to make the two nugets work with one another.
Beta Was this translation helpful? Give feedback.
All reactions
-
Solution A - IQuantity.Value returns QuantityValue seems like a good idea regardless, I think.
Also @pgrawehr wrote:
That's only half the problem. Worse (at least for me now) is that all the conversion properties also return double.
Right, I had not thought of that. I see no reason we shouldn't change those to decimal. Would you be interested in attempting a PR?
UnitsNet.Information.FromKilobytes(5).Bytes; // double, should be decimal
Beta Was this translation helpful? Give feedback.
All reactions
-
@angularsen I can try, but I can't say when I find time. Also, I guess that needs a change to the code generator, which I haven't looked into.
Beta Was this translation helpful? Give feedback.
All reactions
-
@pgrawehr No problem, just let us know if you start on it by creating a PR early and then we can assist you there if you have any questions.
Beta Was this translation helpful? Give feedback.
All reactions
-
@angularsen : I've started looking into this, but would you mind updating the v5 branch with the relevant changes from master? I get a lot of merge conflicts (not counting the generated code) of which I'm unsure how to resolve.
Beta Was this translation helpful? Give feedback.
All reactions
-
@pgrawehr Great, I just merged in master and bumped the release branch to 5.0.0-alpha005.
The merge conflicts can be a pain for sure with all the generated code :-/
Beta Was this translation helpful? Give feedback.
All reactions
-
I think a key point that we should probably also have a discussion about is what would be the ideal internal representation of the value field. Should it be double/decimal, QuantityValue or even maybe something fancier like Fractions?
Beta Was this translation helpful? Give feedback.
All reactions
-
Well, if we go with the Generic solution, it could be user-defined.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
For visibility, I am re-posting this: #875 (reply in thread)
Although generic value types seems nice to have, it is likely that only a minority will make use of it and it adds a lot of complexity to our code base. The mission of Units.NET has from the start been to simplify things for most users, not to cover every use case.
Proposal
Implement solution A - IQuantity.Value returns QuantityValue in the main nuget
- @pgrawehr is looking into it Support for double/decimal values #875 (reply in thread)
- Related, conversion properties like
Information.Bitsshould also returndecimalinstead ofdouble
Implement solution D - Generics with class/record types as a prototype nuget
- Identify 1-3 persons who really wants this feature and become the champions of this feature. Anyone here?
- Build a prototype
- Publish a separate nuget
UnitsNet.Xp.Genericsor similar, to indicate it is experimental and not a primary package
We can then see if the idea gets any traction, both on implementing it and on user adoption.
We can also much faster iterate on the idea without having to think about breaking people's code, and get a better feel of pros and cons.
If it turns out this is something a lot of people have been silently waiting for, or we simply find the improvement worth it ourselves, then it's much easier to bring into the main library later.
What do you guys think?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
Regarding solution A:
If QuantityValue became the internal representation of the quantities, there would be an easy way to cut its size by 60%.
Taking advantage of the fact that decimal does not use bits 0 to 15 (and also bits 24 to 30) of its fourth 32-bit portion, we can construct a union struct that uses the first byte of this unused memory (meaning all zeroes) to save whether a decimal or double is stored (other types, such as long, ulong, float, int etc. could also be supported in this manner). This way, both fields overlap, and moreover, no additional memory for the nullability used to distinguish the underlying types in the current implementation is needed.
Small prototype with only rudimentary operator support and handling of different underlying types:
[StructLayout(LayoutKind.Explicit, Size = 16)]
public readonly struct UnionQuantityValue
{
public UnionQuantityValue(decimal value) : this()
{
Decimal = value;
Type = DataType.Decimal; // not actually necessary due to bit structure of decimal
}
public UnionQuantityValue(double value) : this()
{
Double = value;
Type = DataType.Double;
}
[FieldOffset(0)]
public readonly decimal Decimal;
[FieldOffset(8)]
public readonly double Double;
[FieldOffset(0)]
public readonly DataType Type;
public bool IsDecimal => Type == DataType.Decimal;
public bool IsDouble => Type == DataType.Double;
public static implicit operator decimal(UnionQuantityValue value) => value.Type switch
{
DataType.Decimal => value.Decimal,
DataType.Double => (decimal)value.Double,
_ => throw new NotImplementedException(),
};
public static implicit operator UnionQuantityValue(decimal @decimal) => new(@decimal);
public static implicit operator double(UnionQuantityValue value) => value.Type switch
{
DataType.Decimal => (double)value.Decimal,
DataType.Double => value.Double,
_ => throw new NotImplementedException(),
};
public static implicit operator UnionQuantityValue(double @double) => new(@double);
public static UnionQuantityValue operator +(UnionQuantityValue a, UnionQuantityValue b)
{
ConvertTypesIfNecessary(ref a, ref b);
return a.Type switch
{
DataType.Decimal => a.Decimal + b.Decimal,
DataType.Double => a.Double + b.Double,
_ => throw new NotImplementedException()
};
}
public static UnionQuantityValue operator -(UnionQuantityValue a)
=> a.Type switch
{
DataType.Decimal => -a.Decimal,
DataType.Double => -a.Double,
_ => throw new NotImplementedException(),
};
private static void ConvertTypesIfNecessary(ref UnionQuantityValue a, ref UnionQuantityValue b)
{
if (a.Type == b.Type)
{
return;
}
a = (decimal)a;
b = (decimal)b;
}
public override string ToString()
=> Type switch
{
DataType.Decimal => Decimal.ToString(),
DataType.Double => Double.ToString(),
_ => throw new NotImplementedException(),
};
public enum DataType : byte
{
Decimal = 0,
Double = 1
}
}
Size comparison
Marshal.SizeOf<UnionQuantityValue>(): 16 B
Marshal.SizeOf<UnitsNet.QuantityValue>(): 40 B
Beta Was this translation helpful? Give feedback.
All reactions
-
Today I learned! This is awesome insight.
Some related work on v5 branch relating to QuantityValue: #1074
The internal representation is not changed, so Length still stores double internally, and Information stores decimal. However, IQuantity.Value is now being changed from double to QuantityValue, in order to preserve the original value. The reduced size would benefit there.
I think what you are proposing is a win even if we don't change the internal representation.
Also I don't see any downsides to your idea, since you already covered how other numeric types can be supported too. The only problem would be numeric types too big to fix, such as Complex or various big number representations. We can revisit if that ever becomes a need.
Would you be interested in attempting a pull request against release/v5 branch?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
Beta Was this translation helpful? Give feedback.