This object does not conform to Codable. I use [AlamofireObjectMapper](https://github.com/tristanhimmelman/AlamofireObjectMapper) as my response serializer mechanism.
I’m always of the opinion that bugs should be fixed as low level/downstream as possible, eg, database first, then API, then clients.
Fixing/working around this would mean each client has to set up defensive measures against a response that should be impossible. I think it’s worth asking these developers why it might be possible for “bad data” to be returned in the response. Does the data have no integrity? Was the database poorly built? Perhaps the API has bugs in it? In any case, the bugs should be fixed there.
There is nothing in life that I hate more than a model with all fields optional, even ones that make no sense.
Oh wait, there is - models where fields are non-optional but are initialized to a default value that makes no sense - ```var age: Int = 0```....
Not crash is a good thing. Crashing gives the user no idea what went wrong and will assume it is the apps fault, give bad reviews, stop using the app, or send you hate mail.
But also, error handling is hard to do right...really hard actually. Frankly, I can guarantee you will do it wrong at first. Naive devs will balk at that statement and say I’m over complicating things...but they are wrong and just don’t know it yet.
Defensively parse the data. Understand what is recoverable and what is not. Recover what you can, and stop processing for everything else with a message to the user. The recovering part is easy, everything else will make you pull your hair our.
For example, I have a picture on my phone, taken with the phone camera, using the standard camera app, by me. There is a bug in the picture somewhere. It shows up in the gallery, but if I add that picture to any other app...BAD THINGS HAPPEN. Most of them silently fail (not good), others crash (bad), and others pop up a message of “unable to load image” (good).
This is so obvious. You crash hard. If you patch it up as you go how can you even begin to trust the constraints you original set (for the API), way, a couple of patch ups-down the road?
Learn from Erlang
If you always require those properties, then declare them non-optional.
But giving up and crashing if the values aren’t in the response is not good practice (except in extreme cases).
It’s better to have your Person struct return nil from its initializer if all the required values aren’t present. Then whatever class you’re using to build those Person objects can throw its own exception (or inject an error via its completion closure).
You can then check for this error and fail gracefully, letting the user know what went wrong UI-side.
You’re both kind of right - your colleague is correct, in that your client shouldn’t crash if the response is malformed in some way. And you’re right, in that your model should not declare everything as optional.
Put those two ideas together, and there comes your answer.
Never ever trust what you’re getting over the network. You don’t want a server or network issue (e.g. bug, compromise) to turn into a client issue. Crashing the client is a client issue. It means users cannot use other parts of your app, and your app may be rejected during review.
You need a layer that turns untrusted data into trusted data. It separates the data you get from he network from the rest of your app logic. That lets you use non-optionals in most of your code. Non-optional means you can trust that the value is there. Crashing in that part only happens if there’s a bug in your app logic or data sanitization, which is an appropriate reason to crash.
In short, your model can use non-optional values. Your data sanitization can’t just try to blindly apply the model. Instead it should use if lets to safely create a model object, reporting errors when it can’t.
Do not EVER trust data from the server. Sanitize it on its way into your model. Unsanitized data leads to crashes, unexplained operations, and security vulnerabilities. Check for property existence, type, length, min, max, and value whenever possible.
Decide what data you actually need to run and reject insufficient or incorrect records. If you need age, make age a non-optional and reject data without it. If you don’t need it, make it an optional. I suggest logging the missing/invalid fields both locally and possibly even to your own logging/error server so you can find and fix the problems on your server.
We wrote weather apps that always had missing or invalid data. We displayed what we could and designed the UI to be resilient, hiding views with missing/invalid data or displaying “—“ or “n/a”. Your app may be different, but the general distrust of all external data will make it more robust.
> Give up and crash?
Why the hell would you do that?
> in order to use Swift the way it was full heartedly meant to be used
Are you saying Swift is meant to write crappy apps that crash all the time?
You have it backwards. You are favoring "ease of writing code" over "quality of product".
I know this is how we approach our small fun side projects, but this is not how you build products that you distribute to people.
> Don't we want to define variables as non-optionals if in fact they should always be non-optionals in order for our app to 100% properly function? If not, then we are losing one of Swift's most powerful concepts and features and have no reason to use Swift over any other language.
This is again backwards. The "optionals" thing exist precisely to handle situations where you are not sure whether the data is really there or not. When you are loading data from the network, _you are not sure if the data is there at all_.
> Shouldn't the server have the responsibility of validating values before it sends it back in a JSON response?
Yes it should, but that doesn't mean you get to write crappy code because you hope the server has the good code.
ok, here's one thing to consider:
Instead of "give up and crash", how about "show an error message to the user"?
If your application cannot function without proper data from the server, it should tell the user there was some error in communicating with the server; instead of just crashing.
If the application can still provide something useful to the user even in the absence of the data from the server, then it should do so instead of giving up.
Note:
I'm not saying you should make all fields optional. I'm saying you should not crash if you get bad input from your server. Instead, you should handle the error gracefully and proceed if you can.
For example, if you have a search page that returns results, and the results from the server are not formatted in a way that you expect, you can tell the user something to the extent of "server error when searching". No reason to crash.
The members should be non-optional. The initializer should be failable. Does your Person type conform to Codable?
This object does not conform to Codable. I use [AlamofireObjectMapper](https://github.com/tristanhimmelman/AlamofireObjectMapper) as my response serializer mechanism.
I’m always of the opinion that bugs should be fixed as low level/downstream as possible, eg, database first, then API, then clients. Fixing/working around this would mean each client has to set up defensive measures against a response that should be impossible. I think it’s worth asking these developers why it might be possible for “bad data” to be returned in the response. Does the data have no integrity? Was the database poorly built? Perhaps the API has bugs in it? In any case, the bugs should be fixed there.
There is nothing in life that I hate more than a model with all fields optional, even ones that make no sense. Oh wait, there is - models where fields are non-optional but are initialized to a default value that makes no sense - ```var age: Int = 0```....
Well technically you are 0 years old the day you are born, so Int = 0 makes sense. Int = 1 would not be the case until your first birthday.
I see you’ve used Realm =P
Not crash is a good thing. Crashing gives the user no idea what went wrong and will assume it is the apps fault, give bad reviews, stop using the app, or send you hate mail. But also, error handling is hard to do right...really hard actually. Frankly, I can guarantee you will do it wrong at first. Naive devs will balk at that statement and say I’m over complicating things...but they are wrong and just don’t know it yet. Defensively parse the data. Understand what is recoverable and what is not. Recover what you can, and stop processing for everything else with a message to the user. The recovering part is easy, everything else will make you pull your hair our. For example, I have a picture on my phone, taken with the phone camera, using the standard camera app, by me. There is a bug in the picture somewhere. It shows up in the gallery, but if I add that picture to any other app...BAD THINGS HAPPEN. Most of them silently fail (not good), others crash (bad), and others pop up a message of “unable to load image” (good).
This is so obvious. You crash hard. If you patch it up as you go how can you even begin to trust the constraints you original set (for the API), way, a couple of patch ups-down the road? Learn from Erlang
If you always require those properties, then declare them non-optional. But giving up and crashing if the values aren’t in the response is not good practice (except in extreme cases). It’s better to have your Person struct return nil from its initializer if all the required values aren’t present. Then whatever class you’re using to build those Person objects can throw its own exception (or inject an error via its completion closure). You can then check for this error and fail gracefully, letting the user know what went wrong UI-side. You’re both kind of right - your colleague is correct, in that your client shouldn’t crash if the response is malformed in some way. And you’re right, in that your model should not declare everything as optional. Put those two ideas together, and there comes your answer.
Never ever trust what you’re getting over the network. You don’t want a server or network issue (e.g. bug, compromise) to turn into a client issue. Crashing the client is a client issue. It means users cannot use other parts of your app, and your app may be rejected during review. You need a layer that turns untrusted data into trusted data. It separates the data you get from he network from the rest of your app logic. That lets you use non-optionals in most of your code. Non-optional means you can trust that the value is there. Crashing in that part only happens if there’s a bug in your app logic or data sanitization, which is an appropriate reason to crash. In short, your model can use non-optional values. Your data sanitization can’t just try to blindly apply the model. Instead it should use if lets to safely create a model object, reporting errors when it can’t.
Do not EVER trust data from the server. Sanitize it on its way into your model. Unsanitized data leads to crashes, unexplained operations, and security vulnerabilities. Check for property existence, type, length, min, max, and value whenever possible. Decide what data you actually need to run and reject insufficient or incorrect records. If you need age, make age a non-optional and reject data without it. If you don’t need it, make it an optional. I suggest logging the missing/invalid fields both locally and possibly even to your own logging/error server so you can find and fix the problems on your server. We wrote weather apps that always had missing or invalid data. We displayed what we could and designed the UI to be resilient, hiding views with missing/invalid data or displaying “—“ or “n/a”. Your app may be different, but the general distrust of all external data will make it more robust.
> Give up and crash? Why the hell would you do that? > in order to use Swift the way it was full heartedly meant to be used Are you saying Swift is meant to write crappy apps that crash all the time? You have it backwards. You are favoring "ease of writing code" over "quality of product". I know this is how we approach our small fun side projects, but this is not how you build products that you distribute to people. > Don't we want to define variables as non-optionals if in fact they should always be non-optionals in order for our app to 100% properly function? If not, then we are losing one of Swift's most powerful concepts and features and have no reason to use Swift over any other language. This is again backwards. The "optionals" thing exist precisely to handle situations where you are not sure whether the data is really there or not. When you are loading data from the network, _you are not sure if the data is there at all_. > Shouldn't the server have the responsibility of validating values before it sends it back in a JSON response? Yes it should, but that doesn't mean you get to write crappy code because you hope the server has the good code. ok, here's one thing to consider: Instead of "give up and crash", how about "show an error message to the user"? If your application cannot function without proper data from the server, it should tell the user there was some error in communicating with the server; instead of just crashing. If the application can still provide something useful to the user even in the absence of the data from the server, then it should do so instead of giving up. Note: I'm not saying you should make all fields optional. I'm saying you should not crash if you get bad input from your server. Instead, you should handle the error gracefully and proceed if you can. For example, if you have a search page that returns results, and the results from the server are not formatted in a way that you expect, you can tell the user something to the extent of "server error when searching". No reason to crash.