automerge / automerge-swift-archived

Wrapper around Swift types that can be modified concurrently by different users, and merged again automatically (a CRDT).
MIT License
159 stars 14 forks source link

[-] remove default public initializer for Actor w/ string #46

Closed heckj closed 2 years ago

heckj commented 2 years ago

Resolves #45

Adding constraint on Actor initialized with a string to match RSBackend conditions: that any strings be represented in hexadecimal characters ONLY. Set up with precondition, as we want to consider this a programming error if you've tried to initialize with another string representation.

Note: this only passes all unit tests when merged after #49

lightsprint09 commented 2 years ago

Maybe we can just conform to ExpressibleByStringLiteral in the test target?

heckj commented 2 years ago

Based on the detail from @ept in #48 - I think we do want to re-enable the actorID initializer akin to what we had, and simply make sure that it's always hex as opposed to allowing for an arbitrary string. Since that's probably more hassle to express in the type system - wanted to check what you thought about having a failable initializer here - either returning a nil actor when it fails - or expressing the failure as an exception being thrown (representing a programmer's error). I didn't see much in the way of either in the API so far, so I wasn't sure how you'd prefer to express this sort of thing.

lightsprint09 commented 2 years ago

Might be a fatalError enough. Fail fast and early? Not sure if an optional init or throwing init helps to mitigate such errors