Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesign UnitSystem to support non-SI systems and configurable default units #709

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 20 additions & 20 deletions CodeGen/Generators/UnitsNetGen/QuantityGenerator.cs
Expand Up @@ -183,23 +183,25 @@ private void GenerateInstanceConstructors()
/// <param name=""value"">The numeric value to construct this quantity with.</param>
/// <param name=""unitSystem"">The unit system to create the quantity with.</param>
/// <exception cref=""ArgumentNullException"">The given <see cref=""UnitSystem""/> is null.</exception>
/// <exception cref=""ArgumentException"">No unit was found for the given <see cref=""UnitSystem""/>.</exception>
/// <exception cref=""ArgumentException"">No default unit was found for the given <see cref=""UnitSystem""/>.</exception>
public {_quantity.Name}({_valueType} value, UnitSystem unitSystem)
{{
if(unitSystem == null) throw new ArgumentNullException(nameof(unitSystem));

var unitInfos = Info.GetUnitInfosFor(unitSystem.BaseUnits);
var firstUnitInfo = unitInfos.FirstOrDefault();
if(unitSystem == null) throw new ArgumentNullException(nameof(unitSystem));
");

Writer.WL(_quantity.BaseType == "double"
? @"
_value = Guard.EnsureValidNumber(value, nameof(value));"
_value = Guard.EnsureValidNumber(value, nameof(value));
"
: @"
_value = value;");
Writer.WL(@"
_unit = firstUnitInfo?.Value ?? throw new ArgumentException(""No units were found for the given UnitSystem."", nameof(unitSystem));
}
_value = value;
");

Writer.WL($@"
var defaultUnitInfo = unitSystem.GetDefaultUnitInfo(QuantityType) as UnitInfo<{_unitEnumName}>;

_unit = defaultUnitInfo?.Value ?? throw new ArgumentException(""No default unit was defined for the given UnitSystem."", nameof(unitSystem));
}}
");
}

Expand Down Expand Up @@ -821,13 +823,12 @@ public double As(UnitSystem unitSystem)
if(unitSystem == null)
throw new ArgumentNullException(nameof(unitSystem));

var unitInfos = Info.GetUnitInfosFor(unitSystem.BaseUnits);
var defaultUnitInfo = unitSystem.GetDefaultUnitInfo(QuantityType) as UnitInfo<{_unitEnumName}>;

var firstUnitInfo = unitInfos.FirstOrDefault();
if(firstUnitInfo == null)
throw new ArgumentException(""No units were found for the given UnitSystem."", nameof(unitSystem));
if(defaultUnitInfo == null)
throw new ArgumentException(""No default unit was found for the given UnitSystem."", nameof(unitSystem));

return As(firstUnitInfo.Value);
return As(defaultUnitInfo.Value);
}}

/// <inheritdoc />
Expand Down Expand Up @@ -864,13 +865,12 @@ IQuantity IQuantity.ToUnit(Enum unit)
if(unitSystem == null)
throw new ArgumentNullException(nameof(unitSystem));

var unitInfos = Info.GetUnitInfosFor(unitSystem.BaseUnits);
var defaultUnitInfo = unitSystem.GetDefaultUnitInfo(QuantityType) as UnitInfo<{_unitEnumName}>;

var firstUnitInfo = unitInfos.FirstOrDefault();
if(firstUnitInfo == null)
throw new ArgumentException(""No units were found for the given UnitSystem."", nameof(unitSystem));
if(defaultUnitInfo == null)
throw new ArgumentException(""No default unit was found for the given UnitSystem."", nameof(unitSystem));

return ToUnit(firstUnitInfo.Value);
return ToUnit(defaultUnitInfo.Value);
}}

/// <inheritdoc />
Expand Down
11 changes: 11 additions & 0 deletions UnitsNet.Tests/CustomCode/LengthTests.cs
Expand Up @@ -4,6 +4,7 @@
using Xunit;
using UnitsNet.Units;
using System;
using System.Linq;

namespace UnitsNet.Tests.CustomCode
{
Expand Down Expand Up @@ -186,6 +187,16 @@ public void Constructor_UnitSystemSI_AssignsSIUnit()
Assert.Equal(LengthUnit.Meter, length.Unit);
}

[Fact]
public void Constructor_UnitSystemMySmallSI_AssignsMillimiters()
{
var myDefaultLengthUnit = Length.Info.UnitInfos.First(x => x.Value == LengthUnit.Millimeter);
lipchev marked this conversation as resolved.
Show resolved Hide resolved
var myUnitSystem = UnitSystem.SI.WithDefaultUnit(QuantityType.Length, myDefaultLengthUnit);

var length = new Length(1.0, myUnitSystem);
Assert.Equal(LengthUnit.Millimeter, length.Unit);
}

[Fact]
public void Constructor_UnitSystemWithNoMatchingBaseUnits_ThrowsArgumentException()
{
Expand Down
51 changes: 51 additions & 0 deletions UnitsNet.Tests/UnitSystemTests.cs
Expand Up @@ -2,6 +2,7 @@
// Copyright 2013 Andreas Gullberg Larsen (andreas.larsen84@gmail.com). Maintained at https://github.com/angularsen/UnitsNet.

using System;
using System.Linq;
using UnitsNet.Units;
using Xunit;

Expand Down Expand Up @@ -149,5 +150,55 @@ public void SIUnitSystemHasCorrectBaseUnits()
Assert.Equal(AmountOfSubstanceUnit.Mole, UnitSystem.SI.BaseUnits.Amount);
Assert.Equal(LuminousIntensityUnit.Candela, UnitSystem.SI.BaseUnits.LuminousIntensity);
}

[Fact]
public void GetDefaultUnitInfoThrowsExceptionForUndefinedQuantity()
{
Assert.Throws<ArgumentException>(() => UnitSystem.SI.GetDefaultUnitInfo(QuantityType.Undefined));
}

[Fact]
public void GetDefaultUnitInfoReturnsNullForQuantitiesWithNoDefaultUnits()
{
// TODO do we expect to preserve this behavior?
// AmplitudeRatio might be unitless- but there are (more than one) ways to express ratios.
Assert.Null(UnitSystem.SI.GetDefaultUnitInfo(AmplitudeRatio.QuantityType));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Preserve returning null if no mapping is defined.
  2. AmplitudeRatio should have a mapping defined in SI, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right- so, in theory, the only way to get a null here would be if one creates a custom unit system, explicitly setting the association to null (as presumable the original unit system would have all of its associations defined).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds about right. We should also create some unit tests that breaks as soon as a new quantity is added and if one forgets to add a mapping of default unit for included unit systems such as SI and EnglishEngineering. I don't think we need to add a mapping for default abbreviation, since I think it's natural to default to the first abbreviation defined in JSON as we already do.

}

[Fact]
public void WithDefaultUnitThrowsIfQuantityTypeIsUndefined()
{
Assert.Throws<ArgumentException>(() => UnitSystem.SI.WithDefaultUnit(QuantityType.Undefined, null));
lipchev marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact]
public void WithDefaultUnitUsesOldBaseUnitsIfNotSpecified()
lipchev marked this conversation as resolved.
Show resolved Hide resolved
{
var myDefaultLengthUnit = Length.Info.UnitInfos.First(x => x.Value == LengthUnit.Millimeter);

var newSI = UnitSystem.SI.WithDefaultUnit(QuantityType.Length, myDefaultLengthUnit, (BaseUnits) null);

Assert.Equal(UnitSystem.SI, newSI); // currently comparing using BaseUnits
}

[Theory]
[InlineData(LengthUnit.Undefined, MassUnit.Kilogram, DurationUnit.Second, ElectricCurrentUnit.Ampere, TemperatureUnit.Kelvin, AmountOfSubstanceUnit.Mole, LuminousIntensityUnit.Candela)]
[InlineData(LengthUnit.Meter, MassUnit.Undefined, DurationUnit.Second, ElectricCurrentUnit.Ampere, TemperatureUnit.Kelvin, AmountOfSubstanceUnit.Mole, LuminousIntensityUnit.Candela)]
[InlineData(LengthUnit.Meter, MassUnit.Kilogram, DurationUnit.Undefined, ElectricCurrentUnit.Ampere, TemperatureUnit.Kelvin, AmountOfSubstanceUnit.Mole, LuminousIntensityUnit.Candela)]
[InlineData(LengthUnit.Meter, MassUnit.Kilogram, DurationUnit.Second, ElectricCurrentUnit.Undefined, TemperatureUnit.Kelvin, AmountOfSubstanceUnit.Mole, LuminousIntensityUnit.Candela)]
[InlineData(LengthUnit.Meter, MassUnit.Kilogram, DurationUnit.Second, ElectricCurrentUnit.Ampere, TemperatureUnit.Undefined, AmountOfSubstanceUnit.Mole, LuminousIntensityUnit.Candela)]
[InlineData(LengthUnit.Meter, MassUnit.Kilogram, DurationUnit.Second, ElectricCurrentUnit.Ampere, TemperatureUnit.Kelvin, AmountOfSubstanceUnit.Undefined, LuminousIntensityUnit.Candela)]
[InlineData(LengthUnit.Meter, MassUnit.Kilogram, DurationUnit.Second, ElectricCurrentUnit.Ampere, TemperatureUnit.Kelvin, AmountOfSubstanceUnit.Mole, LuminousIntensityUnit.Undefined)]
public void WithDefaultUnitThrowsIfSpecifiedBaseUnitsNotFullyDefined(LengthUnit length, MassUnit mass, DurationUnit time, ElectricCurrentUnit current,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, aren't we removing BaseUnits?

TemperatureUnit temperature, AmountOfSubstanceUnit amount, LuminousIntensityUnit luminousIntensity)
{
var myDefaultLengthUnit = Length.Info.UnitInfos.First(x => x.Value == LengthUnit.Millimeter);

var baseUnits = new BaseUnits(length, mass, time, current, temperature, amount, luminousIntensity);

// TODO do we want to preserve this behavior?
Assert.Throws<ArgumentException>(()=> UnitSystem.SI.WithDefaultUnit(QuantityType.Length, myDefaultLengthUnit, baseUnits));
}

}
}
30 changes: 14 additions & 16 deletions UnitsNet/GeneratedCode/Quantities/Acceleration.g.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 14 additions & 16 deletions UnitsNet/GeneratedCode/Quantities/AmountOfSubstance.g.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 14 additions & 16 deletions UnitsNet/GeneratedCode/Quantities/AmplitudeRatio.g.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.