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.
