mostly cosmetic modifications

- renamed some tests, added comments here and there
- WithDefaultUnit : added a check for the compatibility between QuantityType and UnitInfo passed in (also added a test for this)

Author: lipchev

UnitsNet.Tests/CustomCode/LengthTests.cs

@@ -188,7 +188,7 @@ public void Constructor_UnitSystemSI_AssignsSIUnit()
         }
 
         [Fact]
-        public void Constructor_UnitSystemMySmallSI_AssignsMillimiters()
+        public void Constructor_UnitSystemWithMillimeters_ConstructsWithMillimeters()
         {
             var myDefaultLengthUnit = Length.Info.UnitInfos.First(x => x.Value == LengthUnit.Millimeter);
             var myUnitSystem = UnitSystem.SI.WithDefaultUnit(QuantityType.Length, myDefaultLengthUnit);

UnitsNet.Tests/UnitSystemTests.cs

@@ -152,27 +152,38 @@ public void SIUnitSystemHasCorrectBaseUnits()
         }
 
         [Fact]
-        public void GetDefaultUnitInfoThrowsExceptionForUndefinedQuantity()
+        public void GetDefaultUnitInfo_GivenUndefinedQuantity_ThrowsArgumentException()
         {
             Assert.Throws<ArgumentException>(() => UnitSystem.SI.GetDefaultUnitInfo(QuantityType.Undefined));
         }
 
         [Fact]
-        public void GetDefaultUnitInfoReturnsNullForQuantitiesWithNoDefaultUnits()
+        public void GetDefaultUnitInfo_GivenQuantityWithNoDefaultUnits_ReturnsNull()
         {
-            // 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));
+            // we cannot simply rely on something like AmplitudeRatio not having a default base unit definition (as this is expected to change soon)
+            var unitSystemWithNoDefaultLengthUnit = UnitSystem.SI.WithDefaultUnit(QuantityType.Length, null); // we can however force the dissociation
+            
+            Assert.Null(unitSystemWithNoDefaultLengthUnit.GetDefaultUnitInfo(QuantityType.Length));
         }
 
         [Fact]
-        public void WithDefaultUnitThrowsIfQuantityTypeIsUndefined()
+        public void WithDefaultUnit_GivenUndefinedQuantityType_ThrowsArgumentException()
         {
-            Assert.Throws<ArgumentException>(() => UnitSystem.SI.WithDefaultUnit(QuantityType.Undefined, null));
+            var anyUnitInfo = Length.Info.UnitInfos.First();
+
+            Assert.Throws<ArgumentException>(() => UnitSystem.SI.WithDefaultUnit(QuantityType.Undefined, anyUnitInfo));
+        }
+
+        [Fact]
+        public void WithDefaultUnit_GivenIncompatibleUnitAndQuantityType_ThrowsArgumentException()
+        {
+            var nonMassUnit = Length.Info.UnitInfos.First();
+
+            Assert.Throws<ArgumentException>(() => UnitSystem.SI.WithDefaultUnit(QuantityType.Mass, nonMassUnit));
         }
 
         [Fact]
-        public void WithDefaultUnitUsesOldBaseUnitsIfNotSpecified()
+        public void WithDefaultUnit_GivenNullForBaseUnits_ReturnsUnitSystemWithOldBaseUnits()
         {
             var myDefaultLengthUnit = Length.Info.UnitInfos.First(x => x.Value == LengthUnit.Millimeter);
 
@@ -189,14 +200,14 @@ public void WithDefaultUnitUsesOldBaseUnitsIfNotSpecified()
         [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,
+        public void WithDefaultUnit_GivenBaseUnitsNotFullyDefined_ThrowsArgumentException(LengthUnit length, MassUnit mass, DurationUnit time, ElectricCurrentUnit current,
             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?
+            // BaseUnits(obsolete) kept in order to avoid introducing  a breaking change (just yet)
             Assert.Throws<ArgumentException>(()=> UnitSystem.SI.WithDefaultUnit(QuantityType.Length, myDefaultLengthUnit, baseUnits));
         }
 

UnitsNet/UnitSystem.cs

@@ -2,9 +2,7 @@
 // Copyright 2013 Andreas Gullberg Larsen ([email protected]). Maintained at https://github.com/angularsen/UnitsNet.
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
-using JetBrains.Annotations;
 using UnitsNet.Units;
 
 namespace UnitsNet
@@ -16,6 +14,7 @@ namespace UnitsNet
     /// </summary>
     public sealed class UnitSystem : IEquatable<UnitSystem>
     {
+        // the array used for storing the default units in the current UnitSystem, ordered by QuantityType (excluding QuantityType.Undefined)
         private readonly Lazy<UnitInfo[]> _defaultUnits;
 
         /// <summary>
@@ -133,7 +132,7 @@ public UnitInfo GetDefaultUnitInfo(QuantityType quantityType)
         {
             if (quantityType == QuantityType.Undefined)
                 throw new ArgumentException("Quantity type can not be undefined.", nameof(quantityType));
-            return _defaultUnits.Value[(int)quantityType - 1];
+            return _defaultUnits.Value[(int) quantityType - 1]; // valid QuantityTypes start from 1 (0 == Undefined)
         }
 
         /// <summary>
@@ -149,18 +148,21 @@ public UnitInfo GetDefaultUnitInfo(QuantityType quantityType)
         ///     <paramref name="quantityType" />
         /// </returns>
         /// <exception cref="ArgumentException">
-        ///     Quantity type can not be undefined.
+        ///     Quantity type can not be undefined and must be compatible with the new default unit (e.g. cannot associate MassUnit with 'Meter')
         /// </exception>
         public UnitSystem WithDefaultUnit(QuantityType quantityType, UnitInfo defaultUnitInfo, BaseUnits baseUnits = null)
         {
-            if (quantityType == QuantityType.Undefined)
+            if (quantityType == QuantityType.Undefined) // redundant with the following test
                 throw new ArgumentException("Quantity type can not be undefined.", nameof(quantityType));
 
-            // TODO any way to check if UnitInfo is of QuantityType?
+            if (defaultUnitInfo != null && !Quantity.Infos.Any(x => x.QuantityType == quantityType && x.UnitInfos.Contains(defaultUnitInfo)))
+                throw new ArgumentException("The unit provided was not found in the list of units for the specified quantity type");
+            
             var newBaseUnits = baseUnits ?? BaseUnits;
 
+            // create a copy of the current mappings, updating only the provided association
             var newDefaultUnits = _defaultUnits.Value.ToArray();
-            newDefaultUnits[(int)quantityType - 1] = defaultUnitInfo;
+            newDefaultUnits[(int) quantityType - 1] = defaultUnitInfo;  // valid QuantityTypes start from 1 (0 == Undefined)
 
             return new UnitSystem(newBaseUnits, newDefaultUnits);
         }