From 80f204d7227c9b3fca997cbc30bb63ef3c9f1ebe Mon Sep 17 00:00:00 2001 From: Philippe Hausler Date: Thu, 4 Aug 2016 10:19:51 -0700 Subject: [PATCH 1/2] [Foundation] Simplify the comparison routines for Measurement The default implementation for comparable automatically adds >, >= etc by having == and < defined. Also the dynamic check for comparison of measurements that are convertible should traverse through the same runtime equality check instead of a static dispatch call out. This resolves: Measurement type defines two '==' functions --- .../public/SDK/Foundation/Measurement.swift | 92 +++---------------- 1 file changed, 15 insertions(+), 77 deletions(-) diff --git a/stdlib/public/SDK/Foundation/Measurement.swift b/stdlib/public/SDK/Foundation/Measurement.swift index 77d5232bc8495..db077514f3154 100644 --- a/stdlib/public/SDK/Foundation/Measurement.swift +++ b/stdlib/public/SDK/Foundation/Measurement.swift @@ -111,19 +111,6 @@ extension Measurement where UnitType : Dimension { } } - /// Compare two measurements of the same `Dimension`. - /// - /// If `lhs.unit == rhs.unit`, returns `lhs.value == rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. - /// - returns: `true` if the measurements are equal. - public static func ==(lhs: Measurement, rhs: Measurement) -> Bool { - if lhs.unit == rhs.unit { - return lhs.value == rhs.value - } else { - let rhsInLhs = rhs.converted(to: lhs.unit) - return lhs.value == rhsInLhs.value - } - } - /// Compare two measurements of the same `Dimension`. /// /// If `lhs.unit == rhs.unit`, returns `lhs.value < rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. @@ -136,45 +123,6 @@ extension Measurement where UnitType : Dimension { return lhs.value < rhsInLhs.value } } - - /// Compare two measurements of the same `Dimension`. - /// - /// If `lhs.unit == rhs.unit`, returns `lhs.value > rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. - /// - returns: `true` if `lhs` is greater than `rhs`. - public static func >(lhs: Measurement, rhs: Measurement) -> Bool { - if lhs.unit == rhs.unit { - return lhs.value > rhs.value - } else { - let rhsInLhs = rhs.converted(to: lhs.unit) - return lhs.value > rhsInLhs.value - } - } - - /// Compare two measurements of the same `Dimension`. - /// - /// If `lhs.unit == rhs.unit`, returns `lhs.value < rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. - /// - returns: `true` if `lhs` is less than or equal to `rhs`. - public static func <=(lhs: Measurement, rhs: Measurement) -> Bool { - if lhs.unit == rhs.unit { - return lhs.value <= rhs.value - } else { - let rhsInLhs = rhs.converted(to: lhs.unit) - return lhs.value <= rhsInLhs.value - } - } - - /// Compare two measurements of the same `Dimension`. - /// - /// If `lhs.unit == rhs.unit`, returns `lhs.value >= rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. - /// - returns: `true` if `lhs` is greater or equal to `rhs`. - public static func >=(lhs: Measurement, rhs: Measurement) -> Bool { - if lhs.unit == rhs.unit { - return lhs.value >= rhs.value - } else { - let rhsInLhs = rhs.converted(to: lhs.unit) - return lhs.value >= rhsInLhs.value - } - } } @available(OSX 10.12, iOS 10.0, watchOS 3.0, tvOS 10.0, *) @@ -225,10 +173,21 @@ extension Measurement { return Measurement(value: lhs / rhs.value, unit: rhs.unit) } - /// Compare two measurements of the same `Unit`. - /// - returns: `true` if `lhs.value == rhs.value && lhs.unit == rhs.unit`. - public static func ==(lhs: Measurement, rhs: Measurement) -> Bool { - return lhs.value == rhs.value && lhs.unit == rhs.unit + /// Compare two measurements of the same `Dimension`. + /// + /// If `lhs.unit == rhs.unit`, returns `lhs.value == rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. + /// - returns: `true` if the measurements are equal. + public static func ==(_ lhs: Measurement, _ rhs: Measurement) -> Bool { + if lhs.unit == rhs.unit { + return lhs.value == rhs.value + } else { + let rhsM = rhs as NSMeasurement + if rhsM.canBeConverted(to: lhs.unit) { + return lhs.value == rhsM.converting(to: lhs.unit).value + } else { + return false + } + } } /// Compare two measurements of the same `Unit`. @@ -237,27 +196,6 @@ extension Measurement { public static func <(lhs: Measurement, rhs: Measurement) -> Bool { return lhs.value < rhs.value } - - /// Compare two measurements of the same `Unit`. - /// - note: This function does not check `==` for the `unit` property of `lhs` and `rhs`. - /// - returns: `lhs.value > rhs.value` - public static func >(lhs: Measurement, rhs: Measurement) -> Bool { - return lhs.value > rhs.value - } - - /// Compare two measurements of the same `Unit`. - /// - note: This function does not check `==` for the `unit` property of `lhs` and `rhs`. - /// - returns: `lhs.value <= rhs.value` - public static func <=(lhs: Measurement, rhs: Measurement) -> Bool { - return lhs.value <= rhs.value - } - - /// Compare two measurements of the same `Unit`. - /// - note: This function does not check `==` for the `unit` property of `lhs` and `rhs`. - /// - returns: `lhs.value >= rhs.value` - public static func >=(lhs: Measurement, rhs: Measurement) -> Bool { - return lhs.value >= rhs.value - } } // Implementation note: similar to NSArray, NSDictionary, etc., NSMeasurement's import as an ObjC generic type is suppressed by the importer. Eventually we will need a more general purpose mechanism to correctly import generic types. From f4333dcebdf264c45549eef6443797b0d9ccd2ea Mon Sep 17 00:00:00 2001 From: Philippe Hausler Date: Mon, 8 Aug 2016 12:36:41 -0700 Subject: [PATCH 2/2] Additional conformance changes for Measurement and unit tests to verify comparison and equality After speaking with the current owner of Measurement, the most sensible path is to at runtime verifty the conformance of Units to a specific dimension since we cannot directly check at compile time. If at a future point in time a more specific comparitor can be added that can restrict the comparison to measurements in a specific dimension without causing equatable failures we may want to revisit this. However as it stands this preserves the most reasonable implementation of comparison of disperate measurement unit types while preserving the expected logic of conversions within that dimension. --- .../public/SDK/Foundation/Measurement.swift | 47 ++++++++++--------- test/1_stdlib/TestMeasurement.swift | 23 +++++++++ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/stdlib/public/SDK/Foundation/Measurement.swift b/stdlib/public/SDK/Foundation/Measurement.swift index db077514f3154..06d9dd5d00ba0 100644 --- a/stdlib/public/SDK/Foundation/Measurement.swift +++ b/stdlib/public/SDK/Foundation/Measurement.swift @@ -110,19 +110,6 @@ extension Measurement where UnitType : Dimension { return Measurement(value: lhsValueInTermsOfBase - rhsValueInTermsOfBase, unit: type(of: lhs.unit).baseUnit()) } } - - /// Compare two measurements of the same `Dimension`. - /// - /// If `lhs.unit == rhs.unit`, returns `lhs.value < rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. - /// - returns: `true` if `lhs` is less than `rhs`. - public static func <(lhs: Measurement, rhs: Measurement) -> Bool { - if lhs.unit == rhs.unit { - return lhs.value < rhs.value - } else { - let rhsInLhs = rhs.converted(to: lhs.unit) - return lhs.value < rhsInLhs.value - } - } } @available(OSX 10.12, iOS 10.0, watchOS 3.0, tvOS 10.0, *) @@ -177,24 +164,38 @@ extension Measurement { /// /// If `lhs.unit == rhs.unit`, returns `lhs.value == rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values. /// - returns: `true` if the measurements are equal. - public static func ==(_ lhs: Measurement, _ rhs: Measurement) -> Bool { + public static func ==(_ lhs: Measurement, _ rhs: Measurement) -> Bool { if lhs.unit == rhs.unit { return lhs.value == rhs.value } else { - let rhsM = rhs as NSMeasurement - if rhsM.canBeConverted(to: lhs.unit) { - return lhs.value == rhsM.converting(to: lhs.unit).value - } else { - return false + if let lhsDimensionalUnit = lhs.unit as? Dimension, + let rhsDimensionalUnit = rhs.unit as? Dimension { + if type(of: lhsDimensionalUnit).baseUnit() == type(of: rhsDimensionalUnit).baseUnit() { + let lhsValueInTermsOfBase = lhsDimensionalUnit.converter.baseUnitValue(fromValue: lhs.value) + let rhsValueInTermsOfBase = rhsDimensionalUnit.converter.baseUnitValue(fromValue: rhs.value) + return lhsValueInTermsOfBase == rhsValueInTermsOfBase + } } + return false } } /// Compare two measurements of the same `Unit`. - /// - note: This function does not check `==` for the `unit` property of `lhs` and `rhs`. - /// - returns: `lhs.value < rhs.value` - public static func <(lhs: Measurement, rhs: Measurement) -> Bool { - return lhs.value < rhs.value + /// - returns: `true` if the measurements can be compared and the `lhs` is less than the `rhs` converted value. + public static func <(lhs: Measurement, rhs: Measurement) -> Bool { + if lhs.unit == rhs.unit { + return lhs.value < rhs.value + } else { + if let lhsDimensionalUnit = lhs.unit as? Dimension, + let rhsDimensionalUnit = rhs.unit as? Dimension { + if type(of: lhsDimensionalUnit).baseUnit() == type(of: rhsDimensionalUnit).baseUnit() { + let lhsValueInTermsOfBase = lhsDimensionalUnit.converter.baseUnitValue(fromValue: lhs.value) + let rhsValueInTermsOfBase = rhsDimensionalUnit.converter.baseUnitValue(fromValue: rhs.value) + return lhsValueInTermsOfBase < rhsValueInTermsOfBase + } + } + fatalError("Attempt to compare measurements with non-equal dimensions") + } } } diff --git a/test/1_stdlib/TestMeasurement.swift b/test/1_stdlib/TestMeasurement.swift index 600edc4d8536d..f4653ddc25cc2 100644 --- a/test/1_stdlib/TestMeasurement.swift +++ b/test/1_stdlib/TestMeasurement.swift @@ -126,6 +126,27 @@ class TestMeasurement : TestMeasurementSuper { // Just make sure we get a result at all here expectFalse(result.isEmpty) } + + func testEquality() { + let fiveKM = Measurement(value: 5, unit: UnitLength.kilometers) + let fiveSeconds = Measurement(value: 5, unit: UnitDuration.seconds) + let fiveThousandM = Measurement(value: 5000, unit: UnitLength.meters) + + expectTrue(fiveKM == fiveThousandM) + expectEqual(fiveKM, fiveThousandM) + expectFalse(fiveKM == fiveSeconds) + } + + func testComparison() { + let fiveKM = Measurement(value: 5, unit: UnitLength.kilometers) + let fiveThousandM = Measurement(value: 5000, unit: UnitLength.meters) + let sixKM = Measurement(value: 6, unit: UnitLength.kilometers) + let sevenThousandM = Measurement(value: 7000, unit: UnitLength.meters) + + expectTrue(fiveKM < sixKM) + expectTrue(fiveKM < sevenThousandM) + expectTrue(fiveKM <= fiveThousandM) + } } #if !FOUNDATION_XCTEST @@ -136,6 +157,8 @@ if #available(OSX 10.12, iOS 10.0, tvOS 10.0, watchOS 3.0, *) { MeasurementTests.test("testOperators") { TestMeasurement().testOperators() } MeasurementTests.test("testUnits") { TestMeasurement().testUnits() } MeasurementTests.test("testMeasurementFormatter") { TestMeasurement().testMeasurementFormatter() } + MeasurementTests.test("testEquality") { TestMeasurement().testEquality() } + MeasurementTests.test("testComparison") { TestMeasurement().testComparison() } runAllTests() } #endif