bkietz commented on code in PR #36496:
URL: https://github.com/apache/arrow/pull/36496#discussion_r1342784537
##########
cpp/src/arrow/util/print.h:
##########
@@ -47,5 +50,27 @@ void PrintTuple(OStream* os, const std::tuple<Args&...>&
tup) {
detail::TuplePrinter<OStream, std::tuple<Args&...>,
sizeof...(Args)>::Print(os, tup);
}
+template <typename Range, typename Separator>
+struct PrintVector {
+ const Range& range_;
+ const Separator& separator_;
+
+ template <typename Os> // template to dodge inclusion of <ostream>
+ friend Os& operator<<(Os& os, PrintVector l) {
+ bool first = true;
+ os << "[";
+ for (const auto& element : l.range_) {
+ if (first) {
+ first = false;
+ } else {
+ os << l.separator_;
+ }
+ os << ToChars(element); // use ToChars to avoid locale dependence
+ }
+ os << "]";
+ return os;
+ }
+};
+
Review Comment:
Adding a template argument deduction guideline for this helper will allow
you to skip specifying them explicitly:
```suggestion
template <typename Range, typename Separator>
PrintVector(const Range&, const Separator&) -> PrintVector<Range, Separator>;
```
##########
cpp/src/arrow/extension/fixed_shape_tensor.cc:
##########
@@ -107,30 +109,16 @@ bool FixedShapeTensorType::ExtensionEquals(const
ExtensionType& other) const {
std::string FixedShapeTensorType::ToString() const {
std::stringstream ss;
ss << "extension<" << this->extension_name()
- << "[value_type=" << value_type_->ToString() << ", shape=[";
- std::string separator;
- for (auto v : shape_) {
- ss << separator << v;
- separator = ",";
- }
- ss << "]";
+ << "[value_type=" << value_type_->ToString() << ", shape="
+ << ::arrow::internal::PrintVector<std::vector<int64_t>,
std::string>{shape_, ","};
Review Comment:
```suggestion
<< ::arrow::internal::PrintVector{shape_, ","};
```
##########
cpp/src/arrow/extension/fixed_shape_tensor.cc:
##########
@@ -107,30 +109,16 @@ bool FixedShapeTensorType::ExtensionEquals(const
ExtensionType& other) const {
std::string FixedShapeTensorType::ToString() const {
std::stringstream ss;
ss << "extension<" << this->extension_name()
- << "[value_type=" << value_type_->ToString() << ", shape=[";
- std::string separator;
- for (auto v : shape_) {
- ss << separator << v;
- separator = ",";
- }
- ss << "]";
+ << "[value_type=" << value_type_->ToString() << ", shape="
+ << ::arrow::internal::PrintVector<std::vector<int64_t>,
std::string>{shape_, ","};
+
if (!permutation_.empty()) {
- ss << ", permutation=[";
- std::string p_separator;
- for (auto v : permutation_) {
- ss << p_separator << v;
- p_separator = ",";
- }
- ss << "]";
+ ss << ", permutation="
+ << ::arrow::internal::PrintVector<std::vector<int64_t>,
std::string>{permutation_,
+
","};
Review Comment:
```suggestion
<< ::arrow::internal::PrintVector{permutation_, ","};
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
