Skip to content

Remove unnecessary symbol export in header files for API management #123

Open
@soblin

Description

Checklist

  • I've read the contribution guidelines.
  • I've searched other issues and no duplicate issues were found.
  • I've agreed with the maintainers that I can plan this task.

Description

Many packages ported from autoware.universe follows the possibly bad practice of inappropriate usage of using in header files. Here is an suspicious example:

using MapProjectorInfo = autoware_map_msgs::msg::MapProjectorInfo;
using GeoPoint = geographic_msgs::msg::GeoPoint;
using LocalPoint = geometry_msgs::msg::Point;

The appropriate usage of using directive in header file is to export the target symbol name as the library module's interface name which all the dependent modules should use when using the library.

Especially this practice is useful if the symbol name originates from a external library,

using Symbol = Foo_v1;

since it makes it easy for us to switch the external library just by renaming the symbol without forcing the library callee side to rename Foo to NewFoo, because if they use Symbol, not Foo in their codebase appropriately, following change remains transparent for them -- which is exactly the role of what's known as API compatibility.

using Symbol = NewFoo;

Below code snippet should remain valid before/after the renaming.

// callee side
void do_stuff(const Symbol & symbol) {
  symbol.call_foo();
}

The issue of current usage

using MapProjectorInfo = autoware_map_msgs::msg::MapProjectorInfo; // I guess it does not make any sense

using GeoPoint = geographic_msgs::msg::GeoPoint; // this seems OK to me, if we ensure that we may redefine `GeoPoint` as a new class in the future and also we promise that all the public API of geographic_msgs::msg::GeoPoint is inherited 
using LocalPoint = geometry_msgs::msg::Point; // seems OK well

is that MapProjectorInfo symbol name is a common identifier in our autoware project and it's very likely that other developers already use fully-qualified autoware_map_msgs::msg::MapProjectorInfo in the same way as autoware::geography_utils::MapProjectorInfo accidentally. Thus it does not serve as a API.

NOTE:

The user should also pass LocalPoint{x=, y=, z=} instead of geometry_msgs::msg::Point{x=, y=, z=}.

Purpose

For removing unnecessary usings in header files.

Possible approaches

We need to check which symbol is our API for customization point and which is not.

Definition of done

Write a guideline in documentation.

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status

      To Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions