sunshinejr / SwiftyUserDefaults

Modern Swift API for NSUserDefaults
http://radex.io/swift/nsuserdefaults/static
MIT License
4.85k stars 365 forks source link

[Suggestion]Change from assertionFailure to fatalError #246

Closed funzin closed 4 years ago

funzin commented 4 years ago

Reference

Summary

This is sample code.

extension Int64: DefaultsSerializable {}
let int64Key = DefaultsKey<Int64>("int64Key", defaultValue: 0)

// not saved
// Defaults[key: int64Key] == 0
Defaults[key: int64Key]= Int64(10000)

When I tried to save the value with the above code, but couldn't save it with Int64.(only iOS11, iOS12) I know that's why JSONEncorder.encode is failing here ref: https://github.com/sunshinejr/SwiftyUserDefaults/blob/master/Sources/Defaults.swift#L75-L80

Especially in the case of Carthage, it is hard to notice because the framework is generated in the release build and it does not crash in the assertionFailure. So I would like to change assertionFailure to fatalError.

funzin commented 4 years ago

@sunshinejr Please check it when you can afford :pray:

sunshinejr commented 4 years ago

hey @funzin, thanks for taking the time and creating a PR - I really appreciate that 💯

in terms of the error here, I understand what you're saying, though I don't really want to crash in that case - I want the framework to give a hint to the developer that it's not actually doing what they want but also not crash on release

this is a hard one because it really is an issue with Codable (types that are Encodable shouldn't ever throw, in my opinion) and also package manager setup

also, for Carthage, you can build your framework in Debug config as well (I agree it might be troublesome if you have a lot of dependencies, though): carthage bootstrap --configuration Debug

thanks again for the PR and hope you understand 🙏

funzin commented 4 years ago

@sunshinejr Thank you for replay. It's just a suggestion, so I'll stand by your opinion:+1:

sunshinejr commented 4 years ago

@funzin thank you!