wvteijlingen / Spine

A Swift library for working with JSON:API APIs. It supports mapping to custom model classes, fetching, advanced querying, linking and persisting.
MIT License
264 stars 109 forks source link

ValueFormatter unformat value return type should be optional. #137

Open markst opened 7 years ago

markst commented 7 years ago

I feel like value formatter return value should be optional.

For example I have a mapping attribute such as: "profilePhotoURL": URLAttribute().serializeAs("profile-image-url") & my response is an empty string for my profile-image-url.

{
  "id": "6c7aee49-1b0d-4e29-95cb-ff3da55e2af6",
  "type": "users",
  "attributes": {
    "name": "Christian Bieniak",
    "phone-number": "0400900410",
    "preferred-name": "Christian",
    "profile-image-url": "",

If URL(string:) fails due to empty string it is then forcefully unwrapped by URLValueFormatter which causes an exception.

wvteijlingen commented 7 years ago

Hmm, I see the problem. I don't know whether unformatting to optionals is the best solution though. I think optionals are only there to support null values, in which case the server response for the attribute should be "null" in the first place. I think it would be better to solve this in the unformatting itself. E.g. returning some fallback value, or throwing a warning.

I don't know the context of this case of course, but it seems to me that the response should actually be "null" for the profile-image-url attribute.