Not happy with a bit of code, anyone suggestions?

Using Swift
1

I need to transform some data that I'm getting from an API into a view model. I have something working, but I'm not happy with it. I'd like some opinions. The code below can be copy/pasted into a playground.

// The data structures that I'm getting back from the API

struct ConnectionConditions {
    var minimumStayCondition: StayCondition?
    var maximumStayCondition: StayCondition?
    var advancePurchaseCondition: StayCondition?
    var cancelCondition: ModifCondition?
    var changeCondition: ModifCondition?
    var noShowCondition: ModifCondition?
}

struct ModifCondition {
    var commercialText: String
    var detailsText: String
    var extraCostsText: String
}

struct StayCondition {
    var commercialText: String
    var detailsText: String
}

// My view model

struct MyViewModel {
    struct RowModel {
        let title: String
        let detailsText: String
    }

    let rowModels: [RowModel]
}

// This is what I came up with

extension MyViewModel {
    init(conditions: [ConnectionConditions]) {
        self.rowModels = conditions.flatMap {
            [
                $0.minimumStayCondition?.makeRowModel(),
                $0.maximumStayCondition?.makeRowModel(),
                $0.advancePurchaseCondition?.makeRowModel(),
                $0.cancelCondition?.makeRowModel(),
                $0.changeCondition?.makeRowModel(),
                $0.noShowCondition?.makeRowModel()
            ].compactMap { $0 }
        }
    }
}

protocol Condition {
    var commercialText: String { get }
    var detailsText: String { get }
    func makeRowModel() -> MyViewModel.RowModel
}

extension Condition {
    func makeRowModel() -> MyViewModel.RowModel {
        MyViewModel.RowModel(title: self.commercialText, detailsText: self.detailsText)
    }
}

extension StayCondition: Condition {}
extension ModifCondition: Condition {}

// Some test data

let testConnectionConditions = [
    ConnectionConditions(minimumStayCondition: nil,
                         maximumStayCondition: StayCondition(commercialText: "LOL", detailsText: "LOLLOL"),
                         advancePurchaseCondition: StayCondition(commercialText: "Monkeys", detailsText: "Oh noes"),
                         cancelCondition: ModifCondition(commercialText: "oh hai", detailsText: "Details", extraCostsText: "Moneysss"),
                         changeCondition: ModifCondition(commercialText: "oops", detailsText: "Yo momma", extraCostsText: "aha"),
                         noShowCondition: ModifCondition(commercialText: "Reddit", detailsText: "oh really", extraCostsText: "actually")),
    ConnectionConditions(minimumStayCondition: StayCondition(commercialText: "whassup", detailsText: "derp"),
                         maximumStayCondition: StayCondition(commercialText: "awesome", detailsText: "camping"),
                         advancePurchaseCondition: StayCondition(commercialText: "braaaains", detailsText: "snacks"),
                         cancelCondition: nil,
                         changeCondition: ModifCondition(commercialText: "haX0rz", detailsText: "zip", extraCostsText: "zap"),
                         noShowCondition: ModifCondition(commercialText: "coding", detailsText: "random", extraCostsText: "ermahgerd"))
]

let myViewModel = MyViewModel(conditions: testConnectionConditions)

So what I did, is adding a protocol and then extending it with the makeRowModel function. I'm not really happy with it. I don't like the "makeRowModel()" but I can't really express why. I also don't like the listing of the six members, like so:

            [
                $0.minimumStayCondition?.makeRowModel(),
                $0.maximumStayCondition?.makeRowModel(),
                $0.advancePurchaseCondition?.makeRowModel(),
                $0.cancelCondition?.makeRowModel(),
                $0.changeCondition?.makeRowModel(),
                $0.noShowCondition?.makeRowModel()
            ]

There must be something more compact, maybe with a KeyPath but I'm not sure whether that'll be better.

2

One way you could make your model initializer more concise is by creating an array of existentials:

([
    $0.minimumStayCondition,
    $0.maximumStayCondition,
    $0.advancePurchaseCondition,
    $0.cancelCondition,
    $0.changeCondition,
    $0.noShowCondition
] as [(any Condition)?]).compactMap { $0?.makeRowModel() }

This exploits the fact that a concrete type wrapped in an optional and then an array, such as [ModifCondition?], can be casted to [(any Condition)?] due Optional's and Array's compiler integration. Also, all your conditions conform to this protocol so this is the way you just specify the condition properties. Perhaps a cleaner way of exposing all conditions is to have a computed property on ConnectionConditions:

extension ConnectionConditions {
  var allConditions: [(any Condition)?] { 
    [minimumStayCondition, maximumStayCondition, /*...*/]
  }
}

If your conditions change rapidly and you can't be bothered to update allConditions each time, you could also use reflection through the Mirror API, but that method is quite slow.

I hope this helps, but let me know if you were referring to a different part of your code you find awkward.

1 Like
3

Not sure if this is any better:

enum ConditionType {
    case minimumStay
    case maximumStay
    case advancePurchase
    case cancel
    case change
    case noShow
}

typealias ConnectionConditions = [ConditionType: Condition]

Edited: although the obvious drawback is that it is very loosely typed and you'd be able associating, say, ModifCondition() value with .minimumStayCondition key and StayCondition() value with .changeCondition key, something not possible in your original implementation.

1 Like
4

enum ConditionType

That's a good solution but that part belongs in the API, which is a bit out-of-scope for me right now. I've logged it as technical debt.

5

This exploits the fact that a concrete type wrapped in an optional and then an array, (...)

Those are very nice solutions, integrating these!

1 Like