Yes, the general guideline is to use Status (in C++ APIs) for exceptional behavior that is out of the control of the user of the C++ APIs. We have been trying to not use Status to make assertions that the developer has invoked the API correctly -- instead preconditions and other such checks should go in DCHECKs.
One consequence of this is that binding layers (e.g. the Cython bindings for Python) need to sanitize user input -- like boundschecking. The codebase isn't entirely consistent about this so far. Some things we could do about this: * Remove the superfluous Status return values and deprecate these APIs in favor of "can't fail" functions * Maintain functions that "can't fail" and ones with extra checks that return Status Happy to hear the opinions of others - Wes On Tue, Mar 20, 2018 at 4:03 PM, Uwe L. Korn <uw...@xhochy.com> wrote: > Hello, > > the general rule is: Use a Status as return value when you would throw an > exception, otherwise return the value directly. In the case where a DCHECK is > sufficient and no method is called that would return a Status, we should get > rid of the Status return. For the AddField case it would be ok to only have a > non-Status variant but we could also provide both methods. > > Uwe > > On Tue, Mar 20, 2018, at 1:16 PM, Antoine Pitrou wrote: >> >> Hello, >> >> I'm trying to understand when it is recommended to return a Status >> instance and use an out parameter for the result value, vs. when it is >> allowed to return the result value naturally. >> >> For instance we currently have the following methods on the Schema class: >> >> Status AddField(int i, const std::shared_ptr<Field>& field, >> std::shared_ptr<Schema>* out) const; >> >> std::shared_ptr<Schema> AddMetadata( >> const std::shared_ptr<const KeyValueMetadata>& metadata) const; >> >> The only reason the Status return is used in `Schema::AddField()` is >> when the user passes an invalid column index `i`, a condition which is >> usually only checked via the debug mode assertions DCHECK_*. >> >> On the one hand, returning the result value results (!) in more natural >> code, both in the callee and in the caller. On the other hand, >> returning a Status allows for finer-grained error reporting later on. >> >> Regards >> >> Antoine.
