Replace force unwrapping with failable initializer #31
Conversation
|
Thinking alike here! This recent PR changed to using It doesn't introduce a failable initializer, so that's definitely something to consider. Do you want to update your PR to just reflect the failable initializer? |
|
Yeah, just squashed a bunch of merge conflicts. Not perfect yet though, still got some issues. |
|
Should be good now. |
|
I see you're advocating for a failable initializer over a force unwrap operator. I see virtue in both - can you let me know why you think failable initializer is the way to go? |
|
When serializing concrete model objects from arbitrary data, it's usually a given that not all data will result in a valid model object. In this case, it makes sense to return nil from the initializer as an indicator of failure, rather than returning a non-nil model object with all of its fields set to nil. In the case where some fields are essential and must not be nil (i.e. a User model fetched from a remote database somewhere might not make sense without its unique id), a failable initializer allows us to express this requirement as a non-optional field without the use of force-unwrapping, which as you know is crashy and frowned upon. The |
|
i have been thinking about this all day; i'm liking the failable initializer for safety. this will come in the release after the latest i've prepared - will ask for a rebase then thanks! ✨ |
hkellaway
changed the title
|
Hm, looks like the code examples in the README may need to be updated as well. |
|
in progress |
|
Replacing this PR with rebased version: #38 |
Better supporting the unhappy path of failure by producing a nil model object instead of a non-nil model object with all of its fields set to nil. This also allows for non-optional model fields without the risk of runtime errors.