Open sakuraehikaru opened 6 years ago
I think that extract function is just too general in it's type signature. It'll run on ANY type, but basically only safely functions on things that are integer-like primitives or a combination of primitives. I think It's not just String and Data, I think you could literally write something like:
let totallyBogus: Bluejay = try bluetoothData.extract(start: 0, length: 6)
One approach to lock it down would be to put some marker protocol (Extractable
, maybe?) just on the primitives that we know can be safely extracted (fixed width Integers and friends) and then restrict the to only act on Extractable
things. That would be the safest thing, but does lose a little bit of useful generality. Right now you could extract a struct or a tuple, for example, which would be safe if it was composed entirely of primitive types, and you'd lose that ability with a marker protocol, though you could at least make structs Extractable by conforming to it yourself.
That could also extend reasonably well to supporting a few more complicated types, if we had:
extension Data {
public func extract<T: Extractable>(start: Int, length: Int) throws -> T {
//...
}
}
To handle all the real primitives, we can then easily add something like:
extension Data {
public func extract(start: Int, length: Int) throws -> Data {
return self.subdata(in: start..<start+length)
}
public func extract(start: Int, length: Int) throws -> String {
return String(data: self.subdata(in: start..<start+length), encoding: .utf8)
}
}
and get reasonable results for String and Data with the same interface.
For 1.0 we're just going to add documentation for this, then look at helpers later on.
Origin of issue: https://github.com/steamclock/bluejay/issues/162
Explanation
I don't think String extraction ever worked. When adding support to extract primitive data types such as various bit-widths of signed and unsigned ints, I probably mistakingly "slapped" on String to the same implementation as well thinking it would just work - but obviously String requires a bit more specialized parsing.
It also turned out we've never had to extract any Strings from any peripherals in our own projects, so this issue remained undiscovered until somebody else actually tried it.
Proposal
We should remove or make it hard for people to try extracting Strings first, then come up with a solution afterwards.