[owncloud-devel] Defensive programming
rmccorkell at karoshi.org.uk
Wed Feb 4 12:21:07 GMT 2015
I feel as though we are in limbo with regards to exceptions. Some code uses
exceptions to report an error, while others return null or false. I suspect
this is one of the reasons why these situations have occurred.
It is my opinion that a function should only return values that make sense.
If a function is named getUser, it should only return the user, and if it
can't that is clearly an error situation where an exception should be
thrown. On the other hand, a function like getListOfThings could viably
return false if there is nothing to get (unless those things are expected
to be there!).
A change to a fully exception based system won't happen overnight, but it'd
be nice to see it promoted more in the code base.
Just my thoughts,
On 4 Feb 2015 12:14, "Vincent Petry" <pvince81 at owncloud.com> wrote:
> After debugging several bugs, I recently found that many pieces of the
> core code is assuming that a function call will always return the
> expected value.
> There were many scenarios where a given bug was causing \OCP::getUser()
> to return null but the code would carry on working with that null value,
> and cause side-effects:
> I suggest that from now on we try and code more defensively.
> This means that we want to check for correct values more often.
> For example when the current user is needed, check for "null" afterwards
> and throw an exception / log a warning instead of letting the code
> Here is another example: https://github.com/owncloud/core/issues/13862
> And here when `getRelativePath()` fails, it is not a normal situation,
> we need to throw an exception instead of continuing:
> What do you think ?
> I have raised https://github.com/owncloud/core/issues/13885 with a list
> of cases that can be fixed. If you know of other places in the code that
> can be improved that way, please add them in the checklist (and/or
> submit a PR to fix them)
> Devel mailing list
> Devel at owncloud.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Devel