[GitHub] [arrow] cyb70289 commented on a change in pull request #11857: ARROW-14907: [C++] Enable CSV Writer to control end-of-line character

[GitBox <gi...@apache.org>]

cyb70289 commented on a change in pull request #11857:
URL: https://github.com/apache/arrow/pull/11857#discussion_r765581649



##########
File path: cpp/src/arrow/csv/writer.cc
##########
@@ -80,17 +80,19 @@ int64_t CountQuotes(util::string_view s) {
 // Matching quote pair character length.
 constexpr int64_t kQuoteCount = 2;
 constexpr int64_t kQuoteDelimiterCount = kQuoteCount + /*end_char*/ 1;
-static const std::string& str_comma = ",";
+constexpr char const* kStrComma = ",";
 
 // Interface for generating CSV data per column.
 // The intended usage is to iteratively call UpdateRowLengths for a column and
 // then PopulateColumns. PopulateColumns must be called in the reverse order of the
 // populators (it populates data backwards).
 class ColumnPopulator {
  public:
-  ColumnPopulator(MemoryPool* pool, const std::string& end_chars,
+  ColumnPopulator(MemoryPool* pool, const std::string end_chars,
                   std::shared_ptr<Buffer> null_string)
-      : end_chars_(end_chars), null_string_(std::move(null_string)), pool_(pool) {}
+      : end_chars_(std::move(end_chars)),

Review comment:
       Should remove the `const`, otherwise there will be data copying.
   But first, why removing the reference?




-- 
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