Merge pull request #1 from NeedleInAJayStack/feature/custom-unit-prot…

…ection

feature: Adds registry protection for custom units

README.md

@@ -65,19 +65,18 @@ conversions.
 
 ### Constants
 
-Units that include a constant value, such as Fahrenheit, cannot be used within composite unit convertions. For example,
+Units that include a constant value, such as Fahrenheit, cannot be used within composite unit conversions. For example,
 you may not convert `5m/°F` to `m/°C` because its unclear how to handle their shifted scale. Instead use the 
 non-shifted Kelvin and Rankine temperature units to refer to temperature differentials.
 
-## Codability
+## Serialization
 
-Each defined unit must have a unique symbol, which is used to identify and encode/decode it. These symbols are not allowed to
+Each defined unit must have a unique symbol, which is used to identify and serialize/deserialize it. These symbols are not allowed to
 contain the `*`, `/`, or `^` characters because those are used in the symbol representation of complex units.
 
 ## Custom Units
 
-To support serialization, Unit is backed by a global registry which is populated with many units by default. However,
-you may add your own custom units to this registry using the `Unit.define` function.
+To extend this package, users can define their own custom units using `Unit.define`:
 
 ```swift
 let centifoot = try Unit.define(
@@ -88,20 +87,47 @@ let centifoot = try Unit.define(
 )
 
 let measurement = Measurement(value: 5, unit: centifoot)
+print(5.measured(in: .foot).convert(to: centifoot))
 ```
 
-Note that you can only define the unit once globally, and afterwards it should be accessed using `Unit(fromSymbol: String)`. 
-If desired, you can simplify access by extending `Unit` with a static property:
+This returns a Unit object that can be used in arithmetic, conversions, and serialization.
+
+### Adding custom units to the Registry
+
+To support deserialization and runtime querying of available units, this package keeps a global
+registry of the default units. The `Unit.define` method does not insert new definitions into this
+registry. While this avoids conflicts and prevents race conditions, it also means that units created
+using `Unit.define` cannot be deserialized correctly or looked up using `Unit(fromSymbol:)` 
+
+If these features are absolutely needed, and the implications are understood, custom units can be
+added to the registry using `Unit.register`:
+
+```swift
+let centifoot = try Unit.register(
+    name: "centifoot",
+    symbol: "cft",
+    dimension: [.Length: 1],
+    coefficient: 0.003048 // This is the conversion to meters
+)
+```
+
+Note that you may only register the unit once globally, and afterwards it should be accessed 
+either by the assigned variable or using `Unit(fromSymbol: String)`.
+
+To simplify access, `Unit` may be extended with a static property:
 
 ```swift
 extension Unit {
-    public static var centifoot = Unit.fromSymbol("cft")
+    public static var centifoot = try! Unit.fromSymbol("cft")
 }
 
 let measurement = 5.measured(in: .centifoot)
 ```
 
-## Future Development Tasks:
+Again, unless strictly necessary, `Unit.define` is preferred over `Unit.register`.
+
+## Contributing
+
+Contributions are absolutely welcome! If you find yourself using a custom unit a lot, feel free
+to stick it in an MR, and we can add it to the default list!
 
-- Add more defined units
-- Allow user-defined quantities

Sources/Units/Registry.swift

@@ -17,6 +17,17 @@ internal class Registry {
             units[defaultUnit.symbol] = defaultUnit
         }
     }
+
+    init(inputUnits: [DefinedUnit]) {
+        units = [:]
+        for inputUnit in inputUnits {
+            // Protect against double-defining symbols
+            if units[inputUnit.symbol] != nil {
+                fatalError("Duplicate symbol: \(inputUnit.symbol)")
+            }
+            units[inputUnit.symbol] = inputUnit
+        }
+    }
 
     /// Returns a list of defined units and their exponents, given a composite unit symbol. It is expected that the caller has
     /// verified that this is a composite unit.

Sources/Units/Unit/Unit.swift

@@ -45,8 +45,15 @@ public struct Unit {
         }
     }
 
-    @discardableResult
-    /// Define a new unit to add to the registry
+    /// Define a unit extension without adding it to the registry. The resulting unit object should be retained
+    /// and passed to the callers that may want to use it.
+    ///
+    /// This unit can be used for arithmatic, conversions, and is encoded correctly. However, since it is
+    /// not part of the global registry it will not be decoded, will not be included in the `allDefined()`
+    /// method, and cannot not be retrieved using `Unit(fromSymbol:)`.
+    ///
+    /// This method is considered "safe" because it does not modify the global unit registry.
+    ///
     /// - Parameters:
     ///   - name: The string name of the unit.
     ///   - symbol: The string symbol of the unit. Symbols may not contain the characters `*`, `/`, or `^`.
@@ -55,13 +62,48 @@ public struct Unit {
     ///   For base units, this is 1.
     ///   - constant: The value to add to a base unit when converting it to this unit. For units without scaling
     ///   differences, this is 0. This is added after the coefficient is multiplied according to order-of-operations.
-    /// - Returns: The unit definition that now exists in the registry.
+    /// - Returns: The unit that was defined.
     public static func define(
         name: String,
         symbol: String,
         dimension: [Quantity: Int],
         coefficient: Double = 1,
         constant: Double = 0
+    ) throws -> Unit {
+        return try Unit(
+            definedBy: .init(
+                name: name,
+                symbol: symbol,
+                dimension: dimension,
+                coefficient: coefficient,
+                constant: constant
+            )
+        )
+    }
+
+    /// **Careful!** Register a new unit to the global registry. Unless you need deserialization support for this unit,
+    /// or support to look up this unit from a different memory-space, we suggest that `define` is used instead.
+    ///
+    /// By using this method, the unit is added to the global registry so it will be deserialized correctly, will be included
+    /// in the `allDefined()` and `Unit(fromSymbol)` methods, and will be available to everyone accessing
+    /// this package in your runtime environment.
+    ///
+    /// - Parameters:
+    ///   - name: The string name of the unit.
+    ///   - symbol: The string symbol of the unit. Symbols may not contain the characters `*`, `/`, or `^`.
+    ///   - dimension: The unit dimensionality as a dictionary of quantities and their respective exponents.
+    ///   - coefficient: The value to multiply a base unit of this dimension when converting it to this unit.
+    ///   For base units, this is 1.
+    ///   - constant: The value to add to a base unit when converting it to this unit. For units without scaling
+    ///   differences, this is 0. This is added after the coefficient is multiplied according to order-of-operations.
+    /// - Returns: The unit definition that now exists in the registry.
+    @discardableResult
+    public static func register(
+        name: String,
+        symbol: String,
+        dimension: [Quantity: Int],
+        coefficient: Double = 1,
+        constant: Double = 0
     ) throws -> Unit {
         try Registry.instance.addUnit(
             name: name,
@@ -348,6 +390,25 @@ public struct Unit {
         case defined(DefinedUnit)
         case composite([DefinedUnit: Int])
     }
+
+    /// Registry singleton that is built up from all Unit static vars and extensions
+    static var registry: Registry {
+        let mirror = Mirror(reflecting: Unit.meter)
+        var units = [DefinedUnit]()
+        print(mirror.children.count)
+        for child in mirror.children {
+            print(child.label)
+            if let unit = child.value as? Unit {
+                switch unit.type {
+                case .defined(let definedUnit):
+                    units.append(definedUnit)
+                case .composite:
+                    break
+                }
+            }
+        }
+        return Registry(inputUnits: units)
+    }
 }
 
 extension Unit: Equatable {

Tests/UnitsTests/MeasurementTests.swift

@@ -185,22 +185,44 @@ final class MeasurementTests: XCTestCase {
         )
     }
 
-    func testCustomUnits() throws {
-        // Test custom defined unit
+    func testUnitDefine() throws {
         let centifoot = try Unit.define(
             name: "centifoot",
             symbol: "cft",
             dimension: [.Length: 1],
             coefficient: 0.003048
         )
-
+
+        // Test conversion from custom unit
         XCTAssertEqual(
-            try 25.measured(in: centifoot).convert(to: .foot),
-            0.25.measured(in: .foot)
+            try 100.measured(in: centifoot).convert(to: .foot),
+            1.measured(in: .foot)
         )
-
-        // Test custom extended unit
-        try Unit.define(
+
+        // Test conversion to custom unit
+        XCTAssertEqual(
+            try 1.measured(in: .foot).convert(to: centifoot).value,
+            100.measured(in: centifoot).value,
+            accuracy: 0.01
+        )
+
+
+        let centiinch = try Unit.define(
+            name: "centiinch",
+            symbol: "cin",
+            dimension: [.Length: 1],
+            coefficient: 0.000254
+        )
+
+        // Test conversion from a custom unit to a different custom unit
+        XCTAssertEqual(
+            try 12.measured(in: centiinch).convert(to: centifoot),
+            1.measured(in: centifoot)
+        )
+    }
+
+    func testUnitRegister() throws {
+        try Unit.register(
             name: "centiinch",
             symbol: "cin",
             dimension: [.Length: 1],

Tests/UnitsTests/UnitTests.swift

@@ -247,4 +247,8 @@ final class UnitTests: XCTestCase {
             try Unit(fromSymbol: "m/2")
         )
     }
+
+    func testDefine() {
+        print(Unit.registry.allUnits())
+    }
 }