From 17acea8f0867d523516253833e80fd38184f4df2 Mon Sep 17 00:00:00 2001 From: Tristan Milnthorp Date: Mon, 15 Oct 2018 15:03:37 -0400 Subject: [PATCH 1/2] Replace GetHashCode implementation to match library. Fixing exceptions with null in methods/operators. Adding tests to validate. --- UnitsNet.Tests/BaseDimensionsTests.cs | 92 ++++++++++++++++++++++++++- UnitsNet/BaseDimensions.cs | 56 +++++++++------- 2 files changed, 122 insertions(+), 26 deletions(-) diff --git a/UnitsNet.Tests/BaseDimensionsTests.cs b/UnitsNet.Tests/BaseDimensionsTests.cs index 0f6f00e675..75cfb95318 100644 --- a/UnitsNet.Tests/BaseDimensionsTests.cs +++ b/UnitsNet.Tests/BaseDimensionsTests.cs @@ -1,17 +1,83 @@ -using Xunit; +using System; +using System.Collections.Generic; +using Xunit; namespace UnitsNet.Tests { public class BaseDimensionsTests { [Fact] - public void EqualityWorksAsExpected() + public void EqualsWorksAsExpected() { var baseDimensions1 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); var baseDimensions2 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); Assert.True(baseDimensions1.Equals(baseDimensions2)); Assert.True(baseDimensions2.Equals(baseDimensions1)); + + Assert.False(baseDimensions1.Equals(null)); + } + + [Fact] + public void EqualityOperatorsWorkAsExpected() + { + var baseDimensions1 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); + var baseDimensions2 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); + + Assert.True(baseDimensions1 == baseDimensions2); + Assert.True(baseDimensions2 == baseDimensions1); + + Assert.False(baseDimensions1 == null); + Assert.False(null == baseDimensions1); + + Assert.False(baseDimensions2 == null); + Assert.False(null == baseDimensions2); + } + + [Fact] + public void InequalityOperatorsWorkAsExpected() + { + var baseDimensions1 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); + var baseDimensions2 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); + + Assert.False(baseDimensions1 != baseDimensions2); + Assert.False(baseDimensions2 != baseDimensions1); + + Assert.True(baseDimensions1 != null); + Assert.True(null != baseDimensions1); + + Assert.True(baseDimensions2 != null); + Assert.True(null != baseDimensions2); + } + + [Fact] + public void MultiplyThrowsExceptionIfPassedNull() + { + var baseDimensions1 = new BaseDimensions(1, 0, 0, 0, 0, 0, 0); + Assert.Throws(() => baseDimensions1.Multiply(null)); + } + + [Fact] + public void DivideThrowsExceptionIfPassedNull() + { + var baseDimensions1 = new BaseDimensions(1, 0, 0, 0, 0, 0, 0); + Assert.Throws(() => baseDimensions1.Divide(null)); + } + + [Fact] + public void MultiplyOperatorThrowsExceptionWithNull() + { + var baseDimensions1 = new BaseDimensions(1, 0, 0, 0, 0, 0, 0); + Assert.Throws(() => baseDimensions1 / null); + Assert.Throws(() => null / baseDimensions1); + } + + [Fact] + public void DivideOperatorThrowsExceptionWithNull() + { + var baseDimensions1 = new BaseDimensions(1, 0, 0, 0, 0, 0, 0); + Assert.Throws(() => baseDimensions1 * null); + Assert.Throws(() => null * baseDimensions1); } [Fact] @@ -575,5 +641,27 @@ public void CheckToStringUsingMolarEntropy() { Assert.Equal("[Length]^2[Mass][Time]^-2[Temperature][Amount]", MolarEntropy.BaseDimensions.ToString()); } + + [Fact] + public void GetHashCodeWorksProperly() + { + var baseDimensions1 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); + var baseDimensions2 = new BaseDimensions(7, 6, 5, 4, 3, 2, 1); + var baseDimensions3 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); + + var hashSet = new HashSet(); + + hashSet.Add(baseDimensions1); + Assert.True(hashSet.Contains(baseDimensions1)); + + hashSet.Add(baseDimensions2); + Assert.True(hashSet.Contains(baseDimensions2)); + + // Should be the same as baseDimensions1 + Assert.True(hashSet.Contains(baseDimensions3)); + + Assert.True(baseDimensions1.GetHashCode() != baseDimensions2.GetHashCode()); + Assert.True(baseDimensions1.GetHashCode() == baseDimensions3.GetHashCode()); + } } } diff --git a/UnitsNet/BaseDimensions.cs b/UnitsNet/BaseDimensions.cs index 7eb558b83d..8f1f0ea7d2 100644 --- a/UnitsNet/BaseDimensions.cs +++ b/UnitsNet/BaseDimensions.cs @@ -47,29 +47,21 @@ public override bool Equals(object obj) if(obj is null || !(obj is BaseDimensions)) return false; - var baseDimensionsObj = (BaseDimensions)obj; - - return Length == baseDimensionsObj.Length && - Mass == baseDimensionsObj.Mass && - Time == baseDimensionsObj.Time && - Current == baseDimensionsObj.Current && - Temperature == baseDimensionsObj.Temperature && - Amount == baseDimensionsObj.Amount && - LuminousIntensity == baseDimensionsObj.LuminousIntensity; + var other = (BaseDimensions)obj; + + return Length == other.Length && + Mass == other.Mass && + Time == other.Time && + Current == other.Current && + Temperature == other.Temperature && + Amount == other.Amount && + LuminousIntensity == other.LuminousIntensity; } /// public override int GetHashCode() { - int hash = 17; - hash = hash * 23 + Length; - hash = hash * 23 + Mass; - hash = hash * 23 + Time; - hash = hash * 23 + Current; - hash = hash * 23 + Temperature; - hash = hash * 23 + Amount; - hash = hash * 23 + LuminousIntensity; - return hash; + return new {Length, Mass, Time, Current, Temperature, Amount, LuminousIntensity}.GetHashCode(); } /// @@ -79,6 +71,9 @@ public override int GetHashCode() /// Resulting dimensions. public BaseDimensions Multiply(BaseDimensions right) { + if(right is null) + throw new ArgumentNullException(nameof(right)); + return new BaseDimensions( Length + right.Length, Mass + right.Mass, @@ -96,6 +91,9 @@ public BaseDimensions Multiply(BaseDimensions right) /// Resulting dimensions. public BaseDimensions Divide(BaseDimensions right) { + if(right is null) + throw new ArgumentNullException(nameof(right)); + return new BaseDimensions( Length - right.Length, Mass - right.Mass, @@ -137,6 +135,11 @@ public BaseDimensions Divide(BaseDimensions right) /// Resulting dimensions. public static BaseDimensions operator *(BaseDimensions left, BaseDimensions right) { + if(left is null) + throw new ArgumentNullException(nameof(left)); + else if(right is null) + throw new ArgumentNullException(nameof(right)); + return left.Multiply(right); } @@ -148,6 +151,11 @@ public BaseDimensions Divide(BaseDimensions right) /// Resulting dimensions. public static BaseDimensions operator /(BaseDimensions left, BaseDimensions right) { + if(left is null) + throw new ArgumentNullException(nameof(left)); + else if(right is null) + throw new ArgumentNullException(nameof(right)); + return left.Divide(right); } #endif @@ -168,16 +176,16 @@ public override string ToString() return sb.ToString(); } - private static void AppendDimensionString( StringBuilder sb, string name, int value ) + private static void AppendDimensionString(StringBuilder sb, string name, int value) { - var absoluteValue = Math.Abs( value ); + var absoluteValue = Math.Abs(value); - if( absoluteValue > 0 ) + if(absoluteValue > 0) { - sb.AppendFormat( "[{0}]", name ); + sb.AppendFormat("[{0}]", name); - if( absoluteValue > 1 ) - sb.AppendFormat( "^{0}", value ); + if(absoluteValue > 1) + sb.AppendFormat("^{0}", value); } } From 283c6b6c4558dd089d7a9f594fb2c3651ec4185e Mon Sep 17 00:00:00 2001 From: Tristan Milnthorp Date: Mon, 15 Oct 2018 15:08:15 -0400 Subject: [PATCH 2/2] Prefer non-equal objects for this test :) --- UnitsNet.Tests/BaseDimensionsTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/UnitsNet.Tests/BaseDimensionsTests.cs b/UnitsNet.Tests/BaseDimensionsTests.cs index 75cfb95318..7ea943e63e 100644 --- a/UnitsNet.Tests/BaseDimensionsTests.cs +++ b/UnitsNet.Tests/BaseDimensionsTests.cs @@ -38,10 +38,10 @@ public void EqualityOperatorsWorkAsExpected() public void InequalityOperatorsWorkAsExpected() { var baseDimensions1 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); - var baseDimensions2 = new BaseDimensions(1, 2, 3, 4, 5, 6, 7); + var baseDimensions2 = new BaseDimensions(7, 6, 5, 4, 3, 2, 1); - Assert.False(baseDimensions1 != baseDimensions2); - Assert.False(baseDimensions2 != baseDimensions1); + Assert.True(baseDimensions1 != baseDimensions2); + Assert.True(baseDimensions2 != baseDimensions1); Assert.True(baseDimensions1 != null); Assert.True(null != baseDimensions1);