Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decoding OptionSets #416

Open
jshier opened this issue Sep 26, 2016 · 7 comments
Open

Decoding OptionSets #416

jshier opened this issue Sep 26, 2016 · 7 comments
Labels

Comments

@jshier
Copy link
Contributor

jshier commented Sep 26, 2016

I'm attempting to parse an API that uses arrays of strings to represent an option's various settings. It seems natural to try and parse this as a type conforming to OptionSet. However, the disconnect between the rawValue and the strings in the array may be a bit too much.

I'm parsing something like this:

{
  "notification_options": ["phone", "email"]
}

Any ideas here? I'm currently going down the path of creating a failable, string-based initializer for my OptionSet but I don't think it's going to be very elegant.

@jshier
Copy link
Contributor Author

jshier commented Sep 27, 2016

So I came up with this code. Surely there's a better way?

let decodedOptions: Decoded<[String]> = json <|| "notification_options"
let settings = decodedOptions.value!
let mapped: [NotificationOptions] = transitions.flatMap(NotificationOptions.init(string:))
let reduced: NotificationOptions = mapped.reduce([]) { (result, transition) -> NotificationOptions in
    var newResult = result
    newResult.insert(transition)

    return newResult
}

Edit. Here's my NotificationOptions type, perhaps there's a better way to express this as well.

struct NotificationOptions: OptionSet {

    let rawValue: Int

    static let entrance = NotificationOptions(rawValue: pushValue)
    static let exit = NotificationOptions(rawValue: smsValue)

    private static let pushValue = 1 << 0
    private static let smsValue = 1 << 1

    init(rawValue: Int) {
        self.rawValue = rawValue
    }

    init?(string: String) {
        switch string {
        case "push":
            rawValue = NotificationOptions.pushValue
        case "sms":
            rawValue = NotificationOptions.smsValue
        default:
            return nil
        }
    }

}

@jshier
Copy link
Contributor Author

jshier commented Sep 27, 2016

I've slightly improved the decoding so that intermediate errors are preserved and turned it into a function that can be flatMap'd.

static func fromStrings(_ strings: [String]) -> Decoded<NotificationOptions> {
    var accumulator: [NotificationOptions] = []
    accumulator.reserveCapacity(strings.count)

    for string in strings {
        switch NotificationOptions(string: string) {
        case .some(let transition):
            accumulator.append(transition)
        case .none:
            return .customError("Failed to create NotificationOptions from string: \(string)")
        }
    }

    let reduced: NotificationOptions = accumulator.reduce([]) { (result, transition) -> NotificationOptions in
        var newResult = result
        newResult.insert(transition)

        return newResult
    }

    return pure(reduced)
}

I'm sure this can be improved.

@jshier
Copy link
Contributor Author

jshier commented Sep 27, 2016

I was able to somewhat simplify my code by creating an intermediate enum that took care of the string decoding for me.

static func fromOptions(_ options: [NotificationOption]) -> Decoded<NotificationOptions> {
    let reduced: NotificationOptions = options.reduce([]) {
        var newResult = $0
        newResult.insert($1.asOption)

        return newResult
    }

    return pure(reduced)
}


enum NotificationOption: String {

    case push
    case sms

    var asOption: NotificationOptions {
        switch self {
        case .push:
            return .push
        case .sms:
            return .sms
        }
    }

}

extension NotificationOption: Decodable { }

struct NotificationOptions: OptionSet {

    let rawValue: Int

    static let push = NotificationOptions(rawValue: 1 << 0)
    static let sms = NotificationOptions(rawValue: 1 << 1)

}

@gfontenot
Copy link
Collaborator

Yeah, this isn't ideal. Sorry for the radio silence on this, I've actually been in not-xcode-land for the last month or so and so have been letting things slip. I'm super interested in finding a nice solution for this, though. Seems like a reasonable use case that isn't well optimized right now.

I wonder if we couldn't cut out the middleman with pattern matching? This is spitballing because Xcode 8 is currently installing for me but maybe something like:

extension NotificationOptions: Decodable {
  static func decode(json: JSON) -> Decoded<NotificationOptions> {
    switch json {
    case .string("push"): return pure(.push)
    case .string("sms"): return pure(.sms)
    default: return .typeMismatch(expected: "NotificationOption", actual: json)
    }
  }
}

And then (I think) you should be able to just use <|| like normal?

Again, I have no compiler available to confirm this for me so sorry if I'm completely off base.

@jshier
Copy link
Contributor Author

jshier commented Nov 4, 2016

I liked the automatic handling of the enum, but your way is certainly more compact. However, I have yet to find a way to generically reduce the resulting array to a single value. I can do it in a typed fashion very easily though.

static func fromOptions(_ options: [NotificationOptions]) -> Decoded< NotificationOptions > {
    return pure(NotificationOptions(options))
}

That can then be flatMapd against the decoded array.

@jshier
Copy link
Contributor Author

jshier commented Nov 4, 2016

While not quite as elegant, I was able to create some generic functions:

func decodedReducedOptions<T: OptionSet>(_ options: [T]) -> Decoded<T> {
    return pure(reducedOptions(options))
}

func reducedOptions<T: OptionSet>(_ options: [T]) -> T {
    return options.reduce(T()) { return $0.union($1) }
}

@gfontenot
Copy link
Collaborator

Oh, right, I guess <|| won't work because it's not decoding to an array. Interesting. I started to write out a solution but it was the same as your generic functions so I bailed. I think that's probably the best option?

I wonder if it'd be useful to introduce an .optionSet static method to Decoded for this? Ideally you'd be able to do something like .optionSet(j <|| "foo") and have it just work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants