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

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

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



##########
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:
       Wouldn't it copy end_chars anyway even if const is removed? Does having const or removing const make any difference?
   
   Yes. The lifetime of end_chars could end early, so as the lifetime of options?
   
   https://github.com/apache/arrow/blob/ec62a45fd65104191385e58d308d6b3c027b7a4c/cpp/src/arrow/csv/writer.cc#L406
   
   
   
   




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