android-library icon indicating copy to clipboard operation
android-library copied to clipboard

`CreationException` is marked as `@Deprecated`.

Open tobiasKaminsky opened this issue 5 years ago • 2 comments
trafficstars

CreationException is marked as @Deprecated.

I think it's rather kludgy to fail with exceptions in general. Many developers don't even know how to handle exceptions or care about handling them, using try-catch block to suppress them (sic!):

try {
    veryImportantThng = makeStuff();
} catch(VeryNastyException ex) {
   log(ex);
}
continue and probably crash later

Maybe we can return a client that fast-fails the operation? This way we can remove a lot of checks in the client code and make it safer:

  1. no edge cases
  2. no null handling

Originally posted by @ezaquarii in https://github.com/nextcloud/android/pull/4900

tobiasKaminsky avatar Dec 02 '19 08:12 tobiasKaminsky

Maybe we can return a client that fast-fails the operation?

I do like the idea. Which ResultCode do we want to use: https://github.com/nextcloud/android-library/blob/0e590238bc32dfcacc6b5cf14b0ea1a6282c8560/src/main/java/com/owncloud/android/lib/common/operations/RemoteOperationResult.java#L84

or a new one, such like "CLIENT_NOT_CONFIGURED"?

tobiasKaminsky avatar Dec 02 '19 08:12 tobiasKaminsky

Maybe we can return a client that fast-fails the operation?

I do like the idea. Which ResultCode do we want to use:

https://github.com/nextcloud/android-library/blob/0e590238bc32dfcacc6b5cf14b0ea1a6282c8560/src/main/java/com/owncloud/android/lib/common/operations/RemoteOperationResult.java#L84

or a new one, such like "CLIENT_NOT_CONFIGURED"?

It is always beneficial to avoid special cases. I'd verify if NO_NETWORK_CONNECTION could natually fit the workflow, as it would also allow us to re-use existing code paths that handle lack of network access.

I'm not sure if this is a good idea, BTW - it might be also beneficial to catch those cases where client cannot be created due to errors in logic.

Otherwise, I'd advocate for CLIENT_NOT_AVAILABLE which is a bit more generic than "configured" (we're not just missing some settings here - there could be more cases like race conditions that cause it).

ezaquarii avatar Dec 05 '19 21:12 ezaquarii