Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CreationException is marked as @Deprecated. #364

Open
tobiasKaminsky opened this issue Dec 2, 2019 · 2 comments
Open

CreationException is marked as @Deprecated. #364

tobiasKaminsky opened this issue Dec 2, 2019 · 2 comments

Comments

@tobiasKaminsky
Copy link
Member

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 nextcloud/android#4900

@tobiasKaminsky
Copy link
Member Author

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

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

or a new one, such like "CLIENT_NOT_CONFIGURED"?

@ezaquarii
Copy link
Contributor

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

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

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants