Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iterate over aggregate fields with the name #171

Merged

Conversation

Baduit
Copy link
Contributor

@Baduit Baduit commented May 14, 2024

Add a new function for_each_field_with_name

The goal is to be able to do this:

struct Toto
{
	int a;
	char c;
};

Toto t {5, 'c'};
auto cb = [](std::string_view name, const auto& value){ std::cout << "Name: " << name << " Value: " << value << std::endl; };
boost::pfr::for_each_field_with_name(t, cb);

I think this is more user friendly than we way it can be done without this PR:

auto cb2 = []<std::size_t Index>(const auto& value, std::integral_constant<std::size_t, Index> i)
{
	std::cout << "Name: " << boost::pfr::get_name<Index, Toto>() << " Value: " << value << std::endl;
};
boost::pfr::for_each_field(t, cb2);

I tried to make the code of for_each_field_with_name as close as possible to for_each_field.

I will add new tests if you think this feature could be merged in the future.

I have explored a bit the idea of completing for_each_field instead of creating a new function in this branch https://github.com/Baduit/pfr/tree/feature/for_each_field_can_provide_name, most of the work is done and it makes possible to do stuff like this:

void plop () {
	std::map<std::string, std::string> m;
	auto fill = [&m](std::string_view name, const auto& value){
		m[std::string(name)] = value;
	};

	boost::pfr::for_each_field(SimpleStruct{ 'e', "test"}, fill);

	assert(m.size() == 2);
	assert(m["c"] == "e");
	assert(m["str"] == "test");
}

void plop2 () {
	std::map<std::string, std::string> m;
	std::map<std::string, std::size_t> mi;
	auto fill = [&m, &mi](std::string_view name, const auto& value, std::size_t i){
		m[std::string(name)] = value;
		mi[std::string(name)] = i;
	};

	boost::pfr::for_each_field(SimpleStruct{ 'e', "test"}, fill);
	assert(m.size() == 2);
	assert(m["c"] == "e");
	assert(m["str"] == "test");
	assert(mi.size() == 2);
	assert(mi["c"] == 0);
	assert(mi["str"] == 1);
}

Do you think it is a better idea ?

@Baduit Baduit changed the title Simpler way to iterate overfield with the value and the name Iterate over aggregate fields with the name May 14, 2024
Copy link
Member

@apolukhin apolukhin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Please see the comments to make the header changes smaller. Also some docs updates are required

/// \endcode
template <class T, class F>
constexpr void for_each_field_with_name(T&& value, F&& func) {
return detail::for_each_field_with_name(std::forward<T>(value), std::forward<F>(func));
Copy link
Member

@apolukhin apolukhin Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be implemented in a simpler way:

return boost::pfr::for_each_field(
    std::forward<T>(value),
    [f = std::forward<F>(func)](auto&& t, auto index) mutable {
        constexpr auto name = boost::pfr::get_name<decltype(index)::value>();
        if constexpr (std::is_invokable_v<F, ?????? >) {
          f(name, std::forward<decltype(t)>(t), index);
        } else {
          f(name, std::forward<decltype(t)>(t));
        }
    });


}}} // namespace boost::pfr::detail

#endif // BOOST_PFR_DETAIL_FOR_EACH_FIELD_WITH_NAME_IMPL_HPP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the documentation file https://github.com/boostorg/pfr/blob/develop/doc/pfr.qbk . Add after pfr_quick_examples_for_each another table row with pfr_quick_examples_for_each_with_name and add pfr_quick_examples_for_each_with_name example to https://github.com/boostorg/pfr/tree/develop/example

@Baduit Baduit force-pushed the feature/foreach_fields_with_names branch 3 times, most recently from 3d2748d to acab7fa Compare June 25, 2024 09:51
@Baduit
Copy link
Contributor Author

Baduit commented Jun 25, 2024

Looks good!

Please see the comments to make the header changes smaller. Also some docs updates are required

I updated my PR according to your comments.

But on the github action I see a failure that I don't understand https://github.com/Baduit/pfr/actions/runs/9660279058/job/26645491447

To be able to iterate over the fields name and value of an aggregate
@Baduit Baduit force-pushed the feature/foreach_fields_with_names branch from acab7fa to c203197 Compare August 27, 2024 21:20
@Baduit
Copy link
Contributor Author

Baduit commented Aug 27, 2024

I updated my branch and thanks to your commit to fix the CI the builds are green

@coveralls
Copy link

Pull Request Test Coverage Report for Build 10586042298

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 10537504895: 0.0%
Covered Lines: 405
Relevant Lines: 405

💛 - Coveralls

@apolukhin apolukhin merged commit 3d090e7 into boostorg:develop Sep 13, 2024
11 checks passed
@apolukhin
Copy link
Member

Many thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants