promotedai / ios-metrics-sdk

iOS client library for Promoted.ai metrics tracking.
MIT License
7 stars 1 forks source link

Make `ClientConfig` a struct so we pass it by value #143

Closed yunapotamus closed 3 years ago

yunapotamus commented 3 years ago

Motivation

ClientConfig should really be a struct, not a class. It was a class previously due to Objective C compatibility, but this approach could lead to mis-use and bugs. This is a clean-up/infrastructure item that I'd like to take care of before releasing the SDK more broadly.

Structs in Swift are pass-by-value, and have the following advantages when using them for objects that hold data:

  1. Assignment always creates a copy, so after doing b = a, any changes to a do not affect b. This prevents clients from changing the ClientConfig after the logging session starts.
  2. Read-only semantics are enforced by the compiler. That is, if you have a let foo = FooStruct(), then you cannot modify any of the fields in foo (ie. foo.bar = "newValue" will not compile). This makes it clear that clients can inspect a configuration object but not modify it. With classes, declaring a readonly object with let foo = FooClass() still allows you to modify its properties, such as foo.bar = "newValue". (The readonly aspect only prevents you from reassigning the foo variable.)

These are difficult to achieve with classes. For 1, you need to make a copy of a class everywhere you want a local immutable instance of it. If you forget, and do a straight assignment without doing a copy, then this can cause difficult-to-diagnose bugs. For 2, you have to make a version of the class with immutable properties, which can duplicate a lot of code.

Copy-on-write wrapper

To achieve the above with minimal boilerplate or duplicate code, we made a struct, ClientConfig, that wraps a class, _ObjCClientConfig. The struct exposes all the properties of the class directly via dynamic member lookup with key paths. This means you can do config.foo on the struct, and access the foo property in the class transparently.

The wrapped instance of the class is kept unique using copy-on-write semantics. This means that copying the struct and modifying the copy does not change the original.

This approach is somewhat unusual, but it has the following advantages.

  1. Structs don't have access to NSObject's key-value coding, so they're missing value() and setValue() methods. We use these methods when merging values from remote configs, so keeping the Objective C object around also preserves this functionality.
  2. Gets us the advantages of the struct while maintaining Objective C compatibility.
  3. Has a lot less boilerplate and duplicate code than other approaches that I investigated (see #142).

ConfigEnums

Removed the ExpressibleByStringLiteral conformance since it wasn't buying us anything in the end. This brings the following improvements:

  1. Don't need an init(stringLiteral:) initializer that has to return a non-nil value. Thus, we can introduce an init?(_ name: String) that can return nil for unsupported names, which is more idiomatic.
  2. Don't need unknown values in the enums.
  3. Less conversion code in ClientConfig+PromotedFirebaseRemoteConfig.

Also added defensive handling of invalid enum values, which resolve to a raw value of 0. Going forward, enums in ClientConfig should always have a case 0, and that case should be the default or a harmless value.