fix handling of rounding to 12 inches in FeetInches

UnitsNet.Tests/CustomCode/LengthTests.FeetInches.cs

@@ -1,22 +1,28 @@
 // Licensed under MIT No Attribution, see LICENSE file at the root.
 // Copyright 2013 Andreas Gullberg Larsen (andreas.larsen84@gmail.com). Maintained at https://github.com/angularsen/UnitsNet.
 
-using System.Collections.Generic;
 using System.Globalization;
-using Xunit;
+using System.Numerics;
+using UnitsNet.Tests.Helpers;
 
 namespace UnitsNet.Tests;
 
 public class FeetInchesTests
 {
-    private const string EnglishUs = "en-US";
-    private const string GermanSwitzerland = "de-CH";
-
+
+    private static readonly CultureInfo EnglishUs = new("en-US", useUserOverride: false);
+    private static readonly CultureInfo GermanSwitzerland = new("de-CH", useUserOverride: false);
+    private const double FeetInOneMeter = 3.28084;
+    private const double InchesInOneMeter = 39.37007874;
+    private const double FeetTolerance = 1e-5;
+    private const double InchesTolerance = 1e-5;
+
     [Fact]
     public void FeetInchesFrom()
     {
-        var meter = Length.FromFeetInches(2, 3);
-        Assert.Equal(0.6858m, meter.Meters);
+        Length meter = Length.FromFeetInches(2, 3);
+        double expectedMeters = 2 / FeetInOneMeter + 3 / InchesInOneMeter;
+        AssertEx.EqualTolerance(expectedMeters, meter.Meters, FeetTolerance);
     }
 
     [Fact]
@@ -73,9 +79,8 @@ public static IEnumerable<object[]> ValidData
 
     [Theory]
     [MemberData(nameof(ValidData))]
-    public void TryParseFeetInches(string str, double expectedFeet, string cultureName)
+    public void TryParseFeetInches(string str, double expectedFeet, CultureInfo formatProvider)
     {
-        var formatProvider = new CultureInfo(cultureName, false);
         Assert.True(Length.TryParseFeetInches(str, out Length result, formatProvider));
         AssertEx.EqualTolerance(expectedFeet, result.Feet, 1e-5);
     }
@@ -105,10 +110,107 @@ public static IEnumerable<object[]> InvalidData
 
     [Theory]
     [MemberData(nameof(InvalidData))]
-    public void TryParseFeetInches_GivenInvalidString_ReturnsFalseAndZeroOut(string str, string cultureName)
+    public void TryParseFeetInches_GivenInvalidString_ReturnsFalseAndZeroOut(string str, CultureInfo formatProvider)
     {
-        var formatProvider = new CultureInfo(cultureName, false);
         Assert.False(Length.TryParseFeetInches(str, out Length result, formatProvider));
         Assert.Equal(Length.Zero, result);
     }
+
+    [Theory]
+    [InlineData(-11.9999, 0, -11.9999)]
+    [InlineData(-23.98, -1, -11.98)]
+    [InlineData(-13, -1, -1)]
+    [InlineData(-38.563, -3, -2.563)]
+
+    public static void NegativeFeetInchesIsAsExpected(double inch, double expectedFeet, double expectedInches)
+    {
+        var length = Length.FromInches(inch);
+
+        Assert.Equal(new System.Numerics.BigInteger(expectedFeet), length.FeetInches.Feet);
+        Assert.Equal(expectedInches, length.FeetInches.Inches, tolerance: 0.000000000001d);
+
+    }
+
+    [Theory]
+    [InlineData(1, -11, 0, 1)]
+    [InlineData(-2, 2, -1, -10)]
+    [InlineData(-1, 32, 1, 8)]
+
+    public static void MixedPositiveNegativeFeetInchesIsAsExpected(long feet, double inch, long expectedFeet, double expectedInches)
+    {
+        var length = Length.FromFeetInches(feet, inch);
+
+        Assert.Equal(new BigInteger(expectedFeet), length.FeetInches.Feet);
+        Assert.Equal(expectedInches, length.FeetInches.Inches.ToDouble());
+
+    }
+
+    [Theory]
+    [InlineData(11.9999, 16, "1' - 0\"")]
+    [InlineData(-11.9999, 16, "-1' - 0\"")]
+    [InlineData(23.98, 32, "1' - 11 31/32\"")]
+    [InlineData(-23.98, 32, "-1' - 11 31/32\"")]
+    [InlineData(13, 32, "1' - 1\"")]
+    [InlineData(-13, 32, "-1' - 1\"")]
+    [InlineData(38.563, 32, "3' - 2 9/16\"")]
+    [InlineData(-38.563, 32, "-3' - 2 9/16\"")]
+    [InlineData(-5.5, 32, "-5 1/2\"")]
+    [InlineData(5.75, 32, "5 3/4\"")]
+    [InlineData(6, 32, "6\"")]
+    [InlineData(-9, 32, "-9\"")]
+    [InlineData(36, 32, "3' - 0\"")]
+    [InlineData(-48, 128, "-4' - 0\"")]
+    [InlineData(-0.5d, 128, "-1/2\"")]
+    [InlineData(0.5d, 128, "1/2\"")]
+    public static void NegativeToArchitecturalString_ReturnsFormatted(double inch, int fractionDenominator, string expected)
+    {
+        var length = Length.FromInches(inch);
+
+        Assert.Equal(expected, length.FeetInches.ToArchitecturalString(fractionDenominator));
+    }
+
+    [Theory]
+    [InlineData(11.9999, "1 ft 0 in")]
+    [InlineData(-11.9999, "-1 ft 0 in")]
+    [InlineData(23.98, "2 ft 0 in")]
+    [InlineData(-23.98, "-2 ft 0 in")]
+    [InlineData(13, "1 ft 1 in")]
+    [InlineData(-13, "-1 ft 1 in")]
+    [InlineData(38.563, "3 ft 3 in")]
+    [InlineData(-38.563, "-3 ft 3 in")]
+    [InlineData(50.2, "4 ft 2 in")]
+    [InlineData(-50.2, "-4 ft 2 in")]
+    [InlineData(-7.6, "-0 ft 8 in")]
+    [InlineData(7.6, "0 ft 8 in")]
+    [InlineData(0, "0 ft 0 in")]
+    [InlineData(-0.0d, "0 ft 0 in")]
+    [InlineData(-50.2, "-4 фут 2 дюйм", "ru-RU")]//ensure we are using alternate units
+    [InlineData(-50.2, "\u22124 ft 2 in", "nb-NO")]// nb-NO does not have alternate abbreviations defined in length.json but does use a different negative symbol        
+    [InlineData(-8, "\u22120 ft 8 in", "nb-NO")]// nb-NO does not have alternate abbreviations defined in length.json but does use a different negative symbol
+    public static void FeetInches_ToStringFormatsCorrectly(double inch, string expected, string? cultureString = null)
+    {
+        var length = Length.FromInches(inch);
+        CultureInfo culture;
+        if (cultureString == null)
+        {
+            culture = CultureInfo.InvariantCulture;
+        }
+        else
+        {
+            culture = new CultureInfo(cultureString, useUserOverride: false);
+        }
+        Assert.Equal(expected, length.FeetInches.ToString(culture));
+    }
+
+    [Theory]
+    [InlineData(47.9988, "4 ft 0 in")]
+    [InlineData(-47.9988, "-4 ft 0 in")]
+    public static void FeetInches_ToString_RoundsTwelveInchesIntoNextFoot(double inch, string expected)
+    {
+        using var _ = new CultureScope(CultureInfo.InvariantCulture);
+
+        var length = Length.FromInches(inch);
+
+        Assert.Equal(expected, length.FeetInches.ToString());
+    }
 }

UnitsNet.Tests/CustomCode/LengthTests.cs

@@ -1,10 +1,7 @@
 // Licensed under MIT No Attribution, see LICENSE file at the root.
 // Copyright 2013 Andreas Gullberg Larsen (andreas.larsen84@gmail.com). Maintained at https://github.com/angularsen/UnitsNet.
 
-using System;
 using System.Globalization;
-using UnitsNet.Units;
-using Xunit;
 
 namespace UnitsNet.Tests
 {
@@ -52,7 +49,7 @@ public class LengthTests : LengthTestsBase
 
         protected override double ShacklesInOneMeter => 0.0364538;
 
-        protected override double NauticalMilesInOneMeter => 1.0/1852.0;
+        protected override double NauticalMilesInOneMeter => 1.0 / 1852.0;
 
         protected override double HandsInOneMeter => 9.8425196850393701;
 
@@ -268,6 +265,8 @@ public static void InverseReturnsReciprocalLength(double value, double expected)
         [InlineData(3, 2.6, 16, "3' - 2 5/8\"")]
         [InlineData(3, 2.6, 32, "3' - 2 19/32\"")]
         [InlineData(3, 2.6, 128, "3' - 2 77/128\"")]
+        [InlineData(3, 11.9988, 128, "4' - 0\"")]
+        [InlineData(0, 0.5d, 128, "1/2\"")]
         public static void ToArchitecturalString_ReturnsFormatted(double ft, double inch, int fractionDenominator, string expected)
         {
             var length = Length.FromFeetInches(ft, inch);

UnitsNet/CustomCode/Quantities/Length.extra.cs

@@ -1,18 +1,16 @@
 // Licensed under MIT No Attribution, see LICENSE file at the root.
 // Copyright 2013 Andreas Gullberg Larsen (andreas.larsen84@gmail.com). Maintained at https://github.com/angularsen/UnitsNet.
 
-using System;
 using System.Globalization;
 using System.Numerics;
 using System.Text.RegularExpressions;
 using System.Threading;
-using UnitsNet.Units;
 
 namespace UnitsNet
 {
     public partial struct Length
     {
-        private static readonly QuantityValue InchesInOneFoot = 12;
+        internal static readonly QuantityValue InchesInOneFoot = 12;
 
         /// <summary>
         ///     Converts the length to a customary feet/inches combination.
@@ -22,7 +20,7 @@ public FeetInches FeetInches
             get
             {
                 QuantityValue totalInches = Inches;
-                return new FeetInches((BigInteger) (totalInches / InchesInOneFoot), totalInches % InchesInOneFoot);
+                return new FeetInches((BigInteger)(totalInches / InchesInOneFoot), totalInches % InchesInOneFoot);
             }
         }
 
@@ -31,7 +29,7 @@ public FeetInches FeetInches
         /// </summary>
         public static Length FromFeetInches(QuantityValue feet, QuantityValue inches)
         {
-            return FromInches(InchesInOneFoot*feet + inches);
+            return FromInches(InchesInOneFoot * feet + inches);
         }
 
         /// <summary>
@@ -46,7 +44,8 @@ public static Length FromFeetInches(QuantityValue feet, QuantityValue inches)
         /// <returns>Parsed length.</returns>
         public static Length ParseFeetInches(string str, IFormatProvider? formatProvider = null)
         {
-            if (str == null) throw new ArgumentNullException(nameof(str));
+            if (str == null)
+                throw new ArgumentNullException(nameof(str));
             if (!TryParseFeetInches(str, out Length result, formatProvider))
             {
                 // A bit lazy, but I didn't want to duplicate this edge case implementation just to get more narrow exception descriptions.
@@ -160,9 +159,34 @@ public string ToString(IFormatProvider? cultureInfo)
             var footUnit = Length.GetAbbreviation(LengthUnit.Foot, unitLocalizationCulture);
             var inchUnit = Length.GetAbbreviation(LengthUnit.Inch, unitLocalizationCulture);
 
+
             // Note that it isn't customary to use fractions - one wouldn't say "I am 5 feet and 4.5 inches".
             // So inches are rounded when converting from base units to feet/inches.
-            return string.Format(cultureInfo, "{0:n0} {1} {2:n0} {3}", Feet, footUnit, Math.Round(Inches.ToDouble()), inchUnit);
+            // When we do this we check if we rounded inches to 12(InchesInOneFoot).
+            // If it does feet/inches are fixed something like 4 ft 0 in is displayed instead of 3ft 12 in for things very close to 4 e.g. 3.9999 ft 
+            BigInteger feet;
+            double inches;
+            var isNegative = Feet < 0 || Inches < 0;
+            var negativeSign = "";
+            if (isNegative)
+            {
+                feet = -Feet;
+                inches = Math.Round(-Inches);
+                negativeSign = (cultureInfo as CultureInfo)?.NumberFormat.NegativeSign ?? "-";
+            }
+            else
+            {
+                feet = Feet;
+                inches = Math.Round(Inches);
+            }
+
+            if (inches == Length.InchesInOneFoot)
+            {
+                feet++;
+                inches = 0;
+            }
+
+            return string.Format(cultureInfo, "{4}{0:n0} {1} {2:n0} {3}", feet, footUnit, inches, inchUnit, negativeSign);
         }
 
         /// <summary>
@@ -190,21 +214,37 @@ public string ToArchitecturalString(int fractionDenominator)
             {
                 throw new ArgumentOutOfRangeException(nameof(fractionDenominator), "Denominator for fractional inch must be greater than zero.");
             }
-
-            // TODO this could probably be done better with the fractions
-            var inchTrunc = (int)Math.Truncate(Inches.ToDouble());
-            var numerator = (int)Math.Round((Inches - inchTrunc).ToDouble() * fractionDenominator); 
+            var feet = Feet;
+            var inches = Inches;
+            //if negative value we record this and invert the values, at the end we add a negative sign as necessary, but all the calculations are done positive so rounding behavior is the same.
+            var isNegative = Feet < 0 || Inches < 0;
+            if (isNegative)
+            {
+                feet = -feet;
+                inches = -inches;
+            }
+
+            var inchTrunc = (int)Math.Truncate(inches);
+            var numerator = (int)Math.Round((inches - inchTrunc) * fractionDenominator);
+
 
             if (numerator == fractionDenominator)
             {
                 inchTrunc++;
                 numerator = 0;
             }
 
+            if (inchTrunc == Length.InchesInOneFoot)
+            {
+                feet++;
+                inchTrunc = 0;
+            }
+
             var inchPart = new System.Text.StringBuilder();
 
             if (inchTrunc != 0 || numerator == 0)
             {
+
                 inchPart.Append(inchTrunc);
             }
 
@@ -235,12 +275,27 @@ static int GreatestCommonDivisor(int a, int b)
 
             inchPart.Append('"');
 
-            if (Feet == 0)
+            if (feet == 0)
             {
+                if (isNegative)
+                {
+                    var negativeSign = CultureInfo.CurrentCulture.NumberFormat.NegativeSign;
+                    //negate inches so the output uses a culture correct negative sign.
+                    inchPart.Insert(0, negativeSign);
+                }
+
                 return inchPart.ToString();
             }
 
-            return $"{Feet}' - {inchPart}";
+            if (isNegative)
+            {
+                //re-negate feet so the output uses a culture correct negative sign.
+                //the behaviour different of feet = -feet  between .netframework and .netcore/.net runtimes,
+                //where newer runtimes support -0.0 is not an issue here because we do not emit the feet part if it is 0.
+                feet = -feet;
+            }
+
+            return $"{feet}' - {inchPart}";
         }
     }
 }