Critique?
#include <iostream>
#include "boost/pfr.hpp"
//////////////////////////////////////////////////////////////////////////////
void print_struct(auto&& s) noexcept
{
using S = std::remove_cvref_t<decltype(s)>;
[&]<auto ...I>(std::index_sequence<I...>) noexcept
{
(
[&]() noexcept
{
if constexpr(I && (I == boost::pfr::tuple_size_v<S> - 1))
{
std::cout << ", " << boost::pfr::get<I>(s) << ')';
}
else if constexpr(I)
{
std::cout << ", " << boost::pfr::get<I>(s);
}
else
{
std::cout << '(' << boost::pfr::get<I>(s);
}
}(),
...
);
}(std::make_index_sequence<boost::pfr::tuple_size_v<S>>());
}
//////////////////////////////////////////////////////////////////////////////
int main()
{
struct
{
int a;
float b;
std::string c;
} s{1, 2.1, "wow"};
print_struct(s);
std::cout << '\n';
return 0;
}
Here's an example.
-
\$\begingroup\$ Have you looked at boost's example? It just misses surrounding parenthesis... \$\endgroup\$Jarod42– Jarod422022年08月23日 08:33:50 +00:00Commented Aug 23, 2022 at 8:33
-
\$\begingroup\$ @Jarod42 Nope, I meant this post to be a variant of the well-known printing-a-tuple problem. \$\endgroup\$user1095108– user10951082022年08月23日 09:20:16 +00:00Commented Aug 23, 2022 at 9:20
2 Answers 2
It only works for some structs
Your print_struct() function only works for types that support aggregate initialization; it's not a perfect generic solution for printing the contents of structs. You might want to add a comment to explain that, and restrict the type of s using a concept so the compiler will be able to print a nice error message.
Simplify
Your code is too complicated; you are handling the start, middle and end of a struct separately, when you only need to handle the first element and the rest separately. It also will not print empty structs correctly. Consider:
void print_struct(auto&& s) noexcept
{
using S = std::remove_cvref_t<decltype(s)>;
std::cout << '(';
[&]<auto ...I>(std::index_sequence<I...>) noexcept
{
(
[&]() noexcept
{
if constexpr(I)
{
std::cout << ", ";
}
std::cout << boost::pfr::get<I>(s);
}(),
...
);
}(std::make_index_sequence<boost::pfr::tuple_size_v<S>>());
std::cout << ')';
}
Handle nested structs
If one of the members of s doesn't have an operator<<() overload for std::ostream, but it is a struct, you could call print_struct() recursively.
Handle printing to other streams
Your print_struct() is hardcoded to print to std::cout, but what if you want to print to std::cerr, a file or a std::stringstream? You can easily support this by adding a reference to std::ostream as a parameter to print_struct().
Incorrect use of noexcept
Since you are printing arbitrary types, there is no guarantee whatsoever that your function won't ever throw, so you should remove noexcept.
Ambiguous output
Simply letting the std::ostream formatter format values does not result in an output is guaranteed to be parsable correctly. Consider the following:
struct
{
int a;
float b;
std::string c;
} s{1, 2, "3, 4); DROP TABLE Students;--"};
print_struct(s);
You might want to choose a serialization format that can be parsed properly, like JSON.
Boost example has already an example for printing aggregate. Format and naming are different, but to adjust to your style, it would be:
template <class T>
void print_struct(const T& x) {
std::cout << "(";
const char* sep = "";
boost::pfr::for_each_field(x, [&](const auto& v) {
std::cout << std::exchange(sep, ", ");
std::cout << v;
});
std::cout << ")";
}
For your version
void print_struct(auto&& s) noexcept
{
using S = std::remove_cvref_t<decltype(s)>;
operator <<might throw, sonoexceptis suspicious.- You dont actually use forwarding reference, const reference would be enough.
and then the
usingwould be unneeded.
->
template <typename S>
void print_struct(const S& s)
{
For
if constexpr(I && (I == boost::pfr::tuple_size_v<S> - 1))
{
std::cout << ", " << boost::pfr::get<I>(s) << ')';
}
else if constexpr(I)
{
std::cout << ", " << boost::pfr::get<I>(s);
}
else
{
std::cout << '(' << boost::pfr::get<I>(s);
}
Your printing is buggy for several cases:
- empty struct print nothing (probably not expected)
- struct with one unique element( prints only
", xx)")
parenthesis should be print outside the fold expression separator should be handled only for first or (exclusively) for last argument.
I prefer the std::exchange version of the separator over doing test on index, but I would say it is mostly just style.
Boost already provides way to iterate on each field (but without the index).
-
\$\begingroup\$ I think
if constexpris superior tostd::exchange(), but anyway, whatever works. `using ̇ is not needed even in this example. \$\endgroup\$user1095108– user10951082022年08月23日 12:08:07 +00:00Commented Aug 23, 2022 at 12:08 -
\$\begingroup\$ @user1095108: I consider both versions as similar and just stylistic difference. (I would bet that performance are similar and time would be taken by
std::cout <<) \$\endgroup\$Jarod42– Jarod422022年08月23日 19:18:04 +00:00Commented Aug 23, 2022 at 19:18 -
\$\begingroup\$ Consider that
boost::pfr::for_each_field()will do something similar to your fold over an index sequence, the compiler will see thatsepis assigned the same pointer in every iteration, so it can remove the redundant stores. \$\endgroup\$G. Sliepen– G. Sliepen2022年08月23日 19:36:47 +00:00Commented Aug 23, 2022 at 19:36