Improve on `KeychainWrapper`

[sbertix]

Hey @jrendel, great job developing this library 💪

I'm migrating my libraries and personal code to rely on your SwiftagramKeychainWrapper, and other than the deprecation warnings suggestion from #159, I feel like the codebase could use some refining in order to feel more "modern", to bring it in line with newer versions of Swift. All changes are additive, maintaining full backwards compatibility.

Changes

• [x] Add inline documentation for all declarations

KeychainItemAccessibility documentation and updates are left to #159, this only deals with KeychainWrapper.

---

• [x] Refactor getters

I agree with your reasoning to both mimic UserDefaults methods, and stick with the "phrasing" of arguments and parameters, but I feel like it could be improved on.

// Something like the code below seems more natural and concise than the previous one.
var data = KeychainWrapper.standard.data(forKey: "key", accessible: .whenUnlocked, synchronized: true)
// And it still retains the previous structure for all common cases.
data = KeychainWrapper.standard.data(forKey: "key")

I also considered the following, but it felt half-baked, and ultimately weird.

let data = KeychainWrapper.standard.data(forKey: "key", accessible: .whenUnlocked, isSynchronizable: true)

There's also some boilerplate code (for instance when guard-ing or if-ing Optionals, to just return the value or nil afterwards) that can be simplified.

---

• [x] Refactor setters (same as above)

You don't actually need specialized methods. One generic function is more than enough 😊

@discardableResult
open func set<T>(_ value: T, forKey key: String, /* … */) -> Bool

---

• [x] Refactor deletion methods (same as above)
• [x] Refactor private methods

---

• [x] Add throwing functions

Adding throwing functions requires thinking of a "prefix"/"suffix" so they do not create conflicts with their Bool-returning counterparts, as I intend to honor backwards compatibility. I wanted to go with fallible, but I felt like non-proficient English speakers, would find themselves looking for "failable" instead. So I opted for unsafe, which still conveys the "throwing" element.

---

• [x] Add Key accessibility and synchronization options

Key values are always stored using default KeychainItemAccessibility and without sync. By adding a custom initializer, we can let the user define the behavior they need.

---

• Consider adding defaultAccessibility and defaultSynchronization

The library already provides for a defaultServiceName, wouldn't it be nice to allow subclasses to also override the default KeychainItemAccessibility and isSynchronizable so they don't need to pass it every single time? If not "out-of-the-box" with KeychainWrapper, with something like ConstrainedKeychainWrapper, subclassing it. An alternative implementation might be instance specific properties, defaulting to nil.

---

Again, please consider this all as just as a suggestion. Thank you again for your work here. 😊

[jrendel]

@sbertix Was there a reason you closed this and the last PR? Sorry I did not take a look earlier. I have not kept up with this repo very well.

I like a lot of what you have done here and will need to spend some time thinking through it. This keychain wrapper was the first thing I did in Swift, back during the beta. So it started off limited by what was available to Swift at the time, and my limited experience with it. Its evolved to what it is now mostly by community efforts and could use a good refactor overall.

[sbertix]

Hey @jrendel, I didn't even realize I closed it but I'm assuming it happened cause I deleted the fork.

I needed all proposed changes above for a project ASAP so I ended up writing a wrapper myself from scratch.

I can work on the PR again though, if you want to, as I think the changes might really benefit people relying on the library.