Change `Variation.treatment` to take a non-optional string

[bjhomer]

Currently, Variation.treatment has an associated value of String?. It looks like this:

public enum Variation: Equatable {
    case control
    case treatment(String?)
}

I'm proposing that it should be this instead:

    case treatment(String)

.treatment(nil) is currently used to represent the default treatment case in a A/B test. You can see here that when the ExPlat server sends back the string "treatment", we represent that as .treatment(nil).

ExPlat now has added beta support for A/B/n experiments, where you can have multiple named variations in addition to the control. This API supports that just fine; you might get .treatment("green_button") or .treatment("blue_button"), and we should definitely continue supporting that.

What I'd like to propose is that we stop mapping "treatment" to nil. Instead, the default A/B variant would simply be .treatment("treatment"). I would argue that doing so is clearer, and no more difficult to work with.

I've checked the other Automattic apps I have access to, and I found only one case so far where this would be a breaking change. That case could easily be updated to check for .treatment("treatment") instead. If we'd like to simplify checking for "is this any kind of treatment at all", we could add the following computed property to Variation:

extension Variation {
  public var isTreatment: Bool {
    switch self {
    case .control: return false
    case .treatment: return true
    }
  }
}

[bjhomer]

Even if you're just doing an A/B test, it's possible to change the name of the default treatment away from "treatment". It just feels to me like having nil there as an option just makes it more confusing. (It also complicates some of the cool experiments code we just rolled out in Day One.)

[mokagio]

It just feels to me like having nil there as an option just makes it more confusing.

I never worked with this, but +1 on .something(.none) being confusing.

Would having three case overcomplicate things?

public enum Variation: Equatable {
    case control
    case defaultTreatment
    case treatment(named: String)
}

[bjhomer]

.defaultTreatment feels a bit verbose to me for something that gets used often, but we could maybe do something like this:

case control
case treatment
case customTreatment(named: String)

[mokagio]

.treatment and .customTreatment(name:) seem like a good option. Nice!

[bjhomer]

@mokagio I've submitted a PR to update the variation representation. This will be a source-breaking change for clients, but only a small one.