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

[C++] Provide a wrapper for invoking factories to produce a Result #23393

Closed
asfimport opened this issue Nov 7, 2019 · 2 comments
Closed

[C++] Provide a wrapper for invoking factories to produce a Result #23393

asfimport opened this issue Nov 7, 2019 · 2 comments

Comments

@asfimport
Copy link
Collaborator

There is a proliferation of code like:

Result<int> SafeAdd(int a, int b) {
  int out;
  RETURN_NOT_OK(SafeAdd(a, b, &out));
  return out;
}

Ideally, this should be resolved by moving the implementation of SafeAdd into the Result returning overload then using Result::Value in the Status returning function. In cases where this is inconvenient, it'd be helpful to have an adapter for doing this more efficiently:

Result<int> SafeAdd(int a, int b) {
  return RESULT_INVOKE(SafeAdd, a, b);
}

This will probably have to be a macro; otherwise the return type can be inferred but only when the function is not overloaded

Reporter: Ben Kietzman / @bkietz
Assignee: Ben Kietzman / @bkietz

Note: This issue was originally created as ARROW-7086. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

Ben Kietzman / @bkietz:
@emkornfield

@asfimport
Copy link
Collaborator Author

Micah Kornfield / @emkornfield:
This seems like potentially the wrong direction to encourage implementations. 

 

I think we should provide the real implementation in the Result<> function and if necessary provide a macro to replace the output value function. e.g. the boiler plate if we want to eliminate it should be:

 

Status SafeAdd(a, b, int* out) {`` 

  ASSIGN_OR_RETURN(auto out, SafeAdd(a, b));

  return Status::OK(); 

It seems like this could be accomplished with template magic but might be OK just as is (or with a template).

 

@asfimport asfimport added this to the 0.16.0 milestone Jan 11, 2023
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