Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove default Decodable extensions #138

Open
Anviking opened this issue Feb 10, 2017 · 2 comments
Open

Remove default Decodable extensions #138

Anviking opened this issue Feb 10, 2017 · 2 comments

Comments

@Anviking
Copy link
Owner

Re: #137

Extensions that can be removed

  • Date
  • URL
  • String
  • Int
  • Double
  • Bool

Extensions that cannot be removed

  • NSDictionary
  • NSArray

These cannot be removed completely, because Array.decoder and others use them. The question here is rather if they should be DynamicDecodable or "fixed".

@voidref
Copy link
Collaborator

voidref commented Feb 11, 2017

I haven't thought about it too much, but it seems a shame to lose a bunch of functionality.

I wonder if they can be separated out and imported piecemeal, as desired, instead.

If we do a big change like this, how many projects is it going to break, how bad will it be, and what can be done to mitigate it.

Something like import DecodableTypeExtensions or something?

@Anviking
Copy link
Owner Author

Anviking commented Feb 11, 2017

I imagined copy-pasting code as an acceptable solution here if the code could be kept short. Maybe there is some problem with this I haven't thought about, but moving the extensions to a different module seems overkill.

For the castable types it's quite simple:

extension String: Castable {}
extension Int: Castable {}
extension Double: Castable {}
extension Bool: Castable {}

For other types, like URL and Date one could do a couple things like

extension Date: Decodable {
    static func decode(_ json: Any) throws -> Date {
        return try formatter.decode(json)
    }
}

and/or adding a helper protocol with default implementation so that this works

extension Date: DecodableFromISO8601DateString {}

but I think it mostly would be obscure and confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants