Functions taken by reference should be const reference

[osrf-migration]

Original report (archived issue) by Trent Houliston (Bitbucket: TrentHouliston).

---

Prerequisites

• Put an X between the brackets on this line if you have done all of the following:

  • Checked the Q&A board for common solutions: http://answers.gazebosim.org
  • Checked that your issue isn't already filed.
  • Checked that there is not already an Ignition package that provides the described functionality: https://ignitionrobotics.org/libs

Description

Functions that take callbacks etc take their callback functions by reference instead of const reference. This prevents lambdas from being used without going through a secondary type. Additionally taking the functions by reference is misleading as you would never change a function object.

I would do a PR to fix this, but I don’t know how deep this rabbit hole would go…

Steps to Reproduce

Compile code that uses a lambda directly in these functions, e.g.

discoveryNode->ConnectionsCb([this](const ignition::transport::MessagePublisher& _publisher) {
    std::cout "Hello world!" << std::endl;
});

Fails to compile as it cannot convert an R value reference into an L value reference.
This could be resolved by taking the reference as const.

Expected behavior:

The code should compile and run as expected.

Actual behavior:

The code fails to compile as you are unable to pass an R value reference into a function that expects an L value reference. There is no reason these functions should be taken by l value reference over const reference which would allow r values to be passed also.

Reproduces how often:

100% of the time

Versions

ign-transport 4.0.0_3
Mac OSX: 10.14.3
Apple LLVM version 10.0.1 (clang-1001.0.46.4)

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

It looks like Discovery::ConnectionsCb for 4.0.0 is already accepting const DiscoveryCallback<Pub> &.

Could you share the exact compilation error that you’re experiencing?