[ 
https://issues.apache.org/jira/browse/ARROW-13956?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Junwang Zhao updated ARROW-13956:
---------------------------------
    Description: 
As Result<T> is encourged to be used, it's better to supply a Marcro to change 
the internal Status.  We could do this by using RETURN_NOT_OK_ELSE, one example 
is:

```

auto reader = arrow::adapters::orc::ORCFileReader::Open(input, pool); 
RETURN_NOT_OK_ELSE(reader, _s.WithMessage("Could not open ORC input source '", 
source.path(), "': ", _s.message()));

return reader;

```

but it's ugly since it use the _s of the macro.

 

Recommend fix:

Add a macro RETURN_NOT_OK_ELSE_WITH_STATUS:

```
 // Use this when you want to change the status in else_ expr
 #define RETURN_NOT_OK_ELSE_WITH_STATUS(s, __s, else_) \
 do { \
 ::arrow::Status _s = ::arrow::internal::GenericToStatus(s); \
 if (!_s.ok()) { \
 else_; \
 return _s; \
 } \
 } while (false)
 ```

And the following statements are more natural

```

auto reader = arrow::adapters::orc::ORCFileReader::Open(input, pool); 
RETURN_NOT_OK_ELSE_WITH_STATUS(reader, status, status.WithMessage("Could not 
open ORC input source '", source.path(), "': ", status.message()));

return reader;

```

  was:
As Result<T> is encourged to be used, it's better to supply a Marcro to change 
the internal Status.  We could do this by using RETURN_NOT_OK_ELSE, one example 
is:

```

auto reader = arrow::adapters::orc::ORCFileReader::Open(input, pool); 
RETURN_NOT_OK_ELSE(reader, _s.WithMessage("Could not open ORC input source '", 
source.path(), "': ", _s.message()));

return reader;

```

but it's ugly since it use the _s of the macro.

 

Recommend fix:

Add a macro RETURN_NOT_OK_ELSE_WITH_STATUS:

```
// Use this when you want to change the status in else_ expr
#define RETURN_NOT_OK_ELSE_WITH_STATUS(s, _s, else_) \
do { \
::arrow::Status _s = ::arrow::internal::GenericToStatus(s); \
if (!_s.ok()) { \
else_; \
return _s; \
} \
} while (false)
```

And the following statements are more natural

```

auto reader = arrow::adapters::orc::ORCFileReader::Open(input, pool); 
RETURN_NOT_OK_ELSE_WITH_STATUS(reader, status, status.WithMessage("Could not 
open ORC input source '", source.path(), "': ", status.message()));

return reader;

```


> Add a RETURN_NOT_OK_ELSE_WITH_STATUS macro to support changing the Status
> -------------------------------------------------------------------------
>
>                 Key: ARROW-13956
>                 URL: https://issues.apache.org/jira/browse/ARROW-13956
>             Project: Apache Arrow
>          Issue Type: Improvement
>          Components: C++
>            Reporter: Junwang Zhao
>            Assignee: Junwang Zhao
>            Priority: Minor
>
> As Result<T> is encourged to be used, it's better to supply a Marcro to 
> change the internal Status.  We could do this by using RETURN_NOT_OK_ELSE, 
> one example is:
> ```
> auto reader = arrow::adapters::orc::ORCFileReader::Open(input, pool); 
> RETURN_NOT_OK_ELSE(reader, _s.WithMessage("Could not open ORC input source 
> '", source.path(), "': ", _s.message()));
> return reader;
> ```
> but it's ugly since it use the _s of the macro.
>  
> Recommend fix:
> Add a macro RETURN_NOT_OK_ELSE_WITH_STATUS:
> ```
>  // Use this when you want to change the status in else_ expr
>  #define RETURN_NOT_OK_ELSE_WITH_STATUS(s, __s, else_) \
>  do { \
>  ::arrow::Status _s = ::arrow::internal::GenericToStatus(s); \
>  if (!_s.ok()) { \
>  else_; \
>  return _s; \
>  } \
>  } while (false)
>  ```
> And the following statements are more natural
> ```
> auto reader = arrow::adapters::orc::ORCFileReader::Open(input, pool); 
> RETURN_NOT_OK_ELSE_WITH_STATUS(reader, status, status.WithMessage("Could not 
> open ORC input source '", source.path(), "': ", status.message()));
> return reader;
> ```



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to