Adopt Codable #127
Adopt Codable #127
Conversation
|
Surprisingly, it turned out to be approximately the same amount of code after adopting Codable and JSONDecoder but still worth it considering improved type safety and cleaner code. Using structs and the same property names on the client and server with default encode/decode implementations would probably have reduced the amount of code a bit more. |
There was a problem hiding this comment.
Cool, I agree that the improved type safety makes it worth landing this refactoring – thanks for taking it on. I wouldn’t worry too much about the increase in line count. If not for accommodating platform naming conventions, this library would be pretty much out of a job. 😉
| code = try container.decodeIfPresent(String.self, forKey: .code)?.uppercased() | ||
| if let identifier = try container.decodeIfPresent(String.self, forKey: .wikidataItemIdentifier) { | ||
| assert(identifier.hasPrefix("Q")) | ||
| wikidataItemIdentifier = identifier |
There was a problem hiding this comment.
Looks like we’re optimistically decoding the QualifyingPlacemark properties in the superclass’s decoder. That’s alright, since GeocodedPlacemark and QualifyingPlacemark don’t have any identically named properties with different implementations. However, it probably points to a need to eventually turn Placemark itself into a protocol.
Fixes #126 adopt Codable along with JSONDecoder
@1ec5 @JThramer @bsudekum 👀