Intermodalics / tango_ros

ROS related software for Tango
Apache License 2.0
67 stars 22 forks source link

Misleading signature in TangoServiceClientNode #308

Closed adamantivm closed 7 years ago

adamantivm commented 7 years ago

The constructor of the TangoServiceClientNode class requires an Activity but immediately casts it to another class TangoServiceClientNode.CallbackListener. This is bad coding practice since it leads developers to build programs that will fail at runtime with cryptic problems, causing long debug times.

This is particularly serious since the documentation explaining how to integrate tango_ros_node in other applications is grossly outdated at the moment.

I suggest making the callback optional and moving it to a separate, explicit setListener call.