Efficiently refactoring piece of Swift code to be less redundant

Question

In the code below, A key is remapped to B key, and vice versa. The remapping is activated via a SwiftUI toggle switch.

---

In example presented here the same block of code is used in three different functions.

Additionally, the loop that iterates through the function call is also used in all three of these functions.

I've been struggling to simplify this code and make it less redundant for more than a day. Any help would be greatly appreciated.

---

let aKey: UInt64 = 0x700000004
let bKey: UInt64 = 0x700000005

func isKeyboardServiceClientForUsagePage(_ serviceClient: IOHIDServiceClient, _ usagePage: UInt32, _ usage: UInt32) -> Bool {
    return IOHIDServiceClientConformsTo(serviceClient, usagePage, usage) == 1
}

func updateKeyboardKeyMapping(_ keyMap: [[String: UInt64]]) {
    let eventSystemClient = IOHIDEventSystemClientCreateSimpleClient(kCFAllocatorDefault)

    guard let serviceClients = IOHIDEventSystemClientCopyServices(eventSystemClient) as? [IOHIDServiceClient] else {
        return
    }

    for serviceClient in serviceClients {
        let usagePage = UInt32(kHIDPage_GenericDesktop)
        let usage = UInt32(kHIDUsage_GD_Keyboard)

        if isKeyboardServiceClientForUsagePage(serviceClient, usagePage, usage) {
            IOHIDServiceClientSetProperty(serviceClient, kIOHIDUserKeyUsageMapKey as CFString, keyMap as CFArray)
        }
    }
}

func areKeysMappedOnAnyServiceClient() -> Bool {
    let eventSystemClient = IOHIDEventSystemClientCreateSimpleClient(kCFAllocatorDefault)

    guard let serviceClients = IOHIDEventSystemClientCopyServices(eventSystemClient) as? [IOHIDServiceClient] else {
        return false
    }

    for serviceClient in serviceClients {
        let usagePage = UInt32(kHIDPage_GenericDesktop)
        let usage = UInt32(kHIDUsage_GD_Keyboard)

        if isKeyboardServiceClientForUsagePage(serviceClient, usagePage, usage) {
            guard let keyMapping = IOHIDServiceClientCopyProperty(serviceClient, kIOHIDUserKeyUsageMapKey as CFString) as? [[String: UInt64]] else {
                return false
            }

            if keyMapping.contains(where: { $0[kIOHIDKeyboardModifierMappingSrcKey] == aKey && $0[kIOHIDKeyboardModifierMappingDstKey] == bKey }) &&
                keyMapping.contains(where: { $0[kIOHIDKeyboardModifierMappingSrcKey] == bKey && $0[kIOHIDKeyboardModifierMappingDstKey] == aKey })
            {
                return true
            }
        }
    }

    return false
}

func remapABBA() {
    let keyMap: [[String: UInt64]] = [
        [
            kIOHIDKeyboardModifierMappingSrcKey: aKey,
            kIOHIDKeyboardModifierMappingDstKey: bKey,
        ],
        [
            kIOHIDKeyboardModifierMappingSrcKey: bKey,
            kIOHIDKeyboardModifierMappingDstKey: aKey,
        ],
    ]

    updateKeyboardKeyMapping(keyMap)
}

func resetKeyMapping() {
    updateKeyboardKeyMapping([])
}

---

And here’s the SwiftUI part if you would like to try the app:

import SwiftUI

struct ContentView: View {
    @State private var remapKeys = areKeysMappedOnAnyServiceClient()

    var body: some View {
        HStack {
            Spacer()
            Toggle(isOn: $remapKeys, label: { Text("Remap A → B and B → A.") })
                .toggleStyle(SwitchToggleStyle())
                .onChange(of: remapKeys, perform: toggleKeyboardRemapping)
            Spacer()
        }
    }
}

private func toggleKeyboardRemapping(_ remapKeys: Bool) {
    if remapKeys {
        remapABBA()
    } else {
        resetKeyMapping()
    }
}

Answer 1

OK... this is going to take some time to answer.

It seems like you're lacking in a place to store things. That's why you have to use the same block of code over and over. We can solve that with a view model...

In here I'm going to hide away the logic of what is happening from the view and only expose what the view needs access to in order to display itself.

// we make it observable so the view can subscribe to it.
class KeyMappingViewModel: ObservableObject {
  private let aKey: UInt64 = 0x700000004
  private let bKey: UInt64 = 0x700000005

  private let srcKey = kIOHIDKeyboardModifierMappingSrcKey
  private let dstKey = kIOHIDKeyboardModifierMappingDstKey

  private var keyMap: [[String: UInt64]] {
    [
      [
        srcKey: aKey,
        dstKey: bKey,
      ],
      [
        srcKey: bKey,
        dstKey: aKey,
      ],
    ]
  }

  // A more concise way to get hold of the client ref
  private var client: IOHIDEventSystemClientRef {
    IOHIDEventSystemClientCreateSimpleClient(kCFAllocatorDefault)
  }

  // Making this published means the view can use it as state in the Toggle
  @Published var toggleState: Bool {
    didSet {
      if toggleState {
        client.updateKeyMapping(keyMap)
      } else {
        client.updateKeyMapping([])
      }
    }
  }

  init() {
    // set the initial value by asking the client if it has any keys mapped
    toggleState = client.areKeysMappedOnAnyServiceClient(aKey: aKey, bKey: bKey)
  }
}

I'm going to make extensions of IOHIDServiceClient and IOHIDEventSystemClientRef to encapsulate your logic...

extension IOHIDEventSystemClientRef {
  private var srcKey: String { kIOHIDKeyboardModifierMappingSrcKey }
  private var dstKey: String { kIOHIDKeyboardModifierMappingDstKey }

  // Make this an optional var on the client ref itself.
  private var serviceClients: [IOHIDServiceClient]? {
    IOHIDEventSystemClientCopyServices(self) as? [IOHIDServiceClient]
  }

  func areKeysMappedOnAnyServiceClient(aKey: UInt64, bKey: UInt64) -> Bool {
    // Nice Swift 5.7 syntax with the optional var
    guard let serviceClients else {
      return false
    }
    // I made this more concise with a filter and map.
    // Also, using the extension we can make use of keyPaths to get the values.
    return serviceClients.filter(\.isForGDKeyboard)
      .compactMap(\.keyMapping)
      .map { keyMapping in
        keyMapping.contains(where: { $0[srcKey] == aKey && $0[dstKey] == bKey }) &&
        keyMapping.contains(where: { $0[srcKey] == bKey && $0[dstKey] == aKey })
      }
      .contains(true)
  }

  func updateKeyMapping(_ keyMap: [[String: UInt64]]) {
    // serviceClients is optional so we can just ? it.
    // if it's nil, nothing after the ? happens.
    serviceClients?.filter(\.isForGDKeyboard)
      .forEach {
        IOHIDServiceClientSetProperty($0, kIOHIDUserKeyUsageMapKey as CFString, keyMap as CFArray)
      }
  }
}

extension IOHIDServiceClient {
  var isForGDKeyboard: Bool {
    let usagePage = UInt32(kHIDPage_GenericDesktop)
    let usage = UInt32(kHIDUsage_GD_Keyboard)
    return IOHIDServiceClientConformsTo(self, usagePage, usage) == 1
  }

  var keyMapping: [[String: UInt64]]? {
    IOHIDServiceClientCopyProperty(self, kIOHIDUserKeyUsageMapKey as CFString) as? [[String: UInt64]]
  }
}

Doing all of this means that your view can look something like this...

import SwiftUI

struct ContentView: View {
  @ObservedObject var viewModel: KeyMappingViewModel = .init()

  var body: some View {
    HStack {
      Spacer()
      Toggle(isOn: $viewModel.toggleState) {
        Text("Remap A → B and B → A.")
      }
      .toggleStyle(SwitchToggleStyle())
      Spacer()
    }
  }
}

This contains all your same logic and TBH wasn't too bad already.

My main changes were to take the free functions and vars and add them to their respective types.

So, the update and areKeysMapped... functions now belong to the IOHIDEventSystemClientRef type.

The isForGDKeyboard and keyMapping vars now belong to the IOHIDServiceClient type.

Doing this removed a lot of the repeated code you had as you no longer had to continuously call free functions. It also meant we unlocked some very Swifty keyPath usage which helped make some of the logic more concise.

Then we made a view model. This allowed us to keep all the moving parts of the view in one place. It had a place to easily get hold of the client. It also meant we could hide a lot of the stuff inside the view model by making it private.

This meant that the view only had one thing it could do. Which is to use the binding to the toggleState. Everything else was behind closed doors to the view.