[GitHub] [arrow] bkietz commented on a diff in pull request #37573: GH-37567: [C++] Migrate JSON Integration code to Result<>

["bkietz (via GitHub)" <gi...@apache.org>]

bkietz commented on code in PR #37573:
URL: https://github.com/apache/arrow/pull/37573#discussion_r1316145229


##########
cpp/src/arrow/testing/json_integration.h:
##########
@@ -114,16 +100,15 @@ class ARROW_TESTING_EXPORT IntegrationJsonReader {
   /// \brief Read a particular record batch from the file
   ///
   /// \param[in] i the record batch index, does not boundscheck
-  /// \param[out] batch the read record batch
-  Status ReadRecordBatch(int i, std::shared_ptr<RecordBatch>* batch) const;
+  /// \return the record batch read
+  Result<std::shared_ptr<RecordBatch>> ReadRecordBatch(int i) const;
 
  private:
-  IntegrationJsonReader(MemoryPool* pool, const std::shared_ptr<Buffer>& data);
+  IntegrationJsonReader(MemoryPool* pool, std::shared_ptr<Buffer> data);

Review Comment:
   Not for this PR, but food for future thought: a pattern which maintains access control for the constructor but enables usage of factories like `make_unique` is the private tag:
   
   ```suggestion
    private:
     class Tag {
       Tag() = default;
       friend class IntegrationJsonReader;
     };
    public:
     IntegrationJsonReader(Tag, MemoryPool* pool, std::shared_ptr<Buffer> data);
   ```
   
   ... probably not worth the boilerplate



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org