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

Prefer lambda to std::bind #1701

Open
iuhilnehc-ynos opened this issue Jun 25, 2021 · 1 comment
Open

Prefer lambda to std::bind #1701

iuhilnehc-ynos opened this issue Jun 25, 2021 · 1 comment
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@iuhilnehc-ynos
Copy link
Collaborator

Feature request

Feature description

Replace all std::bind with lambda. Is it worth doing it for now?

isocpp/CppCoreGuidelines#1796

Implementation considerations

e.g.

Replace

parameter_subscription_ = rclcpp::AsyncParametersClient::on_parameter_event(
node_topics_,
std::bind(&TimeSource::on_parameter_event, this, std::placeholders::_1));
}
with

  parameter_subscription_ = rclcpp::AsyncParametersClient::on_parameter_event(
    node_topics_,
    [this](std::shared_ptr<const rcl_interfaces::msg::ParameterEvent> event) {
        return on_parameter_event(event);
      }
    );
@wjwwood
Copy link
Member

wjwwood commented Jun 25, 2021

I'm not opposed to it, but I'm also not sure if it is a really important thing to do either.

A few somewhat unimportant things to consider:

  • using std::bind in rclcpp gives us implicit coverage on that use case, which is nice but not required as we have demos that cover that case too
  • in the example you gave, std::bind is slightly nicer because we don't need to include the type of event, and don't think auto would work in this case due to how the template deduction works on that

But I don't think those points push me one way or the other that much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants