search

saoudrizwan / Disk

Easily persist structs, images, and data on iOS
MIT License
3.1k stars 170 forks source link

Support for error handling #2

Closed nmdias closed 7 years ago

nmdias commented 7 years ago

Please consider propagating errors to the user so that they can be handled at runtime.

saoudrizwan commented 7 years ago

There's a version of Disk where all the methods throw errors, so then developers would have to use do, catch, try wherever they use Disk. For the sake of simplicity, I thought it may be best to just keep things safe and cancel store(:in:as:) requests before printing an error log, and return nil if an error occurs during retrieval.

I'm open to any suggestions to changing the way Disk handles errors though, please let me know what you think.

nmdias commented 7 years ago

Not necessarily, if the developer would like to opt out of the Error handling he could just do an optional try?. I know it would add complexity, and make it more verbose, but it seems odd not having error handling in IO operations.

Of the top of my head, I would simply propagate the errors thrown by the FileManager.

As an alternative, maybe if the user could provide it's own custom logger. As it is, while it's possible, it would probably start to raise concerns on weather a singleton should exist, replacing the static methods that are in place. No override is possible at this time. This way the user could know why it failed.

But I guess at the end of the day, while returning nil is an option so that some proper feedback could be sent to the user, if you were to send this information back to the server, that information would be limited to telling that something failed instead of why it failed.

Congrats on the framework by the way. Very classy 🥇

saoudrizwan commented 7 years ago

You make some really good points. I'll spend the night thinking about it, and I might just revert Disk back to a bunch of throwing functions. You're right that it's best giving the developer more access to information about the IO operations. I guess I just wanted to make Disk's syntax look nicer than it should haha.

nmdias commented 7 years ago

Great, let me know if you need help or a pull request with whatever you decide.

🙏

saoudrizwan commented 7 years ago

@nmdias I pulled an all-nighter refactoring Disk. You were right, I should give developers more access to the errors, so now Disk is extremely thorough when it comes to error handling. I opted to use throwing functions instead of just returning nil or cancelling an operation. Thank you for your input, it actually helped me a lot!