additional constructor for collisionrequest is a bad idea

[humanoid-path-planner]

additional constructor for collisionrequest is a bad idea

[florent-lamiraux]

If you are true, there should be a compilation warning.

[jmirabel]

I would be in favor of using flags rather than constructors with a series of booleans.
I find CollisionRequest (Contact | DistanceLowerBound, 1) more readable than CollisionRequest (true, 1, true)...

[stonneau]

Well if you create a collision request you should see a warning stating that max_num_contacts is 0. I spotted it in the rb-rrt test of hpp-rbrm, because I was expecting collisions that did not occur. ps: depending on the compiler you don't always get a compilation warning in such cases, in particular with default constructors. See for instance a more complex case: http://www.gotw.ca/gotw/005.htm

…

________________________________ De : Joseph Mirabel <[email protected]> Envoyé : mardi 8 janvier 2019 13:49 À : humanoid-path-planner/hpp-fcl Cc : stonneau; Author Objet : Re: [humanoid-path-planner/hpp-fcl] additional constructor for collisionrequest is a bad idea (#39) I would be in favor of using flags rather than constructors with a series of booleans. I find CollisionRequest (Contact | DistanceLowerBound, 1) more readable than CollisionRequest (true, 1, true)... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#39 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEaCppJQ_q82coEOQe0jlIW7WnEtKu9Lks5vBJPhgaJpZM4Z1Xrk>.

[florent-lamiraux]

I meant that the constructor that is called is deprecated. The compiler should complain or call the other one.

[stonneau]

In this case my pull request should not be merged.

…

________________________________ De : Florent Lamiraux <[email protected]> Envoyé : mardi 8 janvier 2019 13:55 À : humanoid-path-planner/hpp-fcl Cc : stonneau; Author Objet : Re: [humanoid-path-planner/hpp-fcl] additional constructor for collisionrequest is a bad idea (#39) I meant that the constructor that is called is deprecated. The compiler should complain or call the other one. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#39 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEaCptep9LcRE3uNgZxiE5IqCMcUc1Avks5vBJU7gaJpZM4Z1Xrk>.

[florent-lamiraux]

I agree with Joseph.

[stonneau]

I'd agree as well, although this drives us further away from the original fcl api.
This is not only about an hypothetic merge, but also with respect to the rest of the api that does not follow this principle.

In any case, such modifications should be conducted on another branch than devel, until the api change is actually confirmed.

[stonneau]

Here it is:
#54