1. Projects
  2. autoware.universe
  3. #2459422

Delta analysis

Reviewed on Jun 10, 10:30

Review level:
View on GitHub
Code Health
9.10
(-0.9)
Code Health Quality gate
Passed
Declining Code Health
3 findings
Improving Code Health
0 findings
Affected Hotspots
0 Files
Authors

SI

Declining Code Health
3 FINDINGS
(3 types in 1 file)
Files with highest declining Code Health findings are at the top.

RoutingAdaptor::on_timer has a cyclomatic complexity of 18, threshold = 9

A Complex Method has a high cyclomatic complexity. The recommended threshold for the C++ language is a cyclomatic complexity lower than 9.

There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring.

This code health issue has been solved before in this project. Here are some examples for inspiration:

shapes.cpp (e69b61df)
(+0.90)
...
Author: Taekjin LEE <taekjin.lee@tier4.jp>
Date: 2025-03-16 15:01+00:00
refactor(multi_object_tracker): internal message driven process (#10203)
* refactor(multi_object_tracker): streamline input channel configuration handling feat(multi_object_tracker): introduce InputChannel struct for input channel configuration refactor(multi_object_tracker): improve marker handling and initialization in TrackerObjectDebugger feat(multi_object_tracker): enhance InputChannel with trust flags for object properties refactor(multi_object_tracker): remove unused channel_size parameter from tracker constructors feat(multi_object_tracker): update InputChannel flags to trust object extension and classification fix(multi_object_tracker): replace channel.index with channel_index for consistency feat(multi_object_tracker): update TrackerObjectDebugger and TrackerProcessor to accept channels_config parameter refactor(multi_object_tracker): remove redundant existence probability initialization from tracker constructors feat(multi_object_tracker): integrate data association into TrackerProcessor and add associate method feat(multi_object_tracker): enhance updateWithMeasurement to include channel_info for improved classification handling refactor(multi_object_tracker): replace object_id with uuid in DynamicObject and related classes fix(multi_object_tracker): update UUID handling in Tracker to use uuid_msg for consistency refactor(multi_object_tracker): simplify pose and covariance handling in tracker classes refactor(multi_object_tracker): replace pose_with_covariance with separate pose and covariance attributes in DynamicObject refactor: remove z state from tracker. it will uses object state refactor(multi_object_tracker): streamline object handling in trackers and remove unnecessary shape processing refactor(multi_object_tracker): remove z position handling from trackers and update object kinematics structure refactor(multi_object_tracker): remove BoundingBox structure from trackers and implement object extension limits refactor(multi_object_tracker): remove unnecessary blank lines in tracker getTrackedObject methods refactor(multi_object_tracker): simplify input channel configuration by removing trust flags and consolidating parameters Signed-off-by: Taekjin LEE <taekjin.lee@tier4.jp> * refactor(multi_object_tracker): use const reference in loop and simplify tracker update logic Signed-off-by: Taekjin LEE <taekjin.lee@tier4.jp> * refactor(multi_object_tracker): update shape handling and streamline object tracking logic Signed-off-by: Taekjin LEE <taekjin.lee@tier4.jp> * refactor(multi_object_tracker): update shape handling to use geometry_msgs::msg::Point for anchor vectors Signed-off-by: Taekjin LEE <taekjin.lee@tier4.jp> * style(pre-commit): autofix Signed-off-by: Taekjin LEE <taekjin.lee@tier4.jp> * refactor(multi_object_tracker): modify getNearestCornerOrSurface function signature and update related logic Signed-off-by: Taekjin LEE <taekjin.lee@tier4.jp> refactor(multi_object_tracker): remove self_transform parameter from measure and update methods refactor(multi_object_tracker): update calcAnchorPointOffset function signature and streamline object handling refactor(multi_object_tracker): set shape type to BOUNDING_BOX for object trackers --------- Signed-off-by: Taekjin LEE <taekjin.lee@tier4.jp> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@@ -13,5 +13,2 @@
// limitations under the License.
//
//
// Author: v1.0 Yukihiro Saito, Taekjin Lee
@@ -69,7 +66,5 @@ double get2dIoU(
const auto source_polygon = autoware_utils::to_polygon2d(
source_object.kinematics.pose_with_covariance.pose, source_object.shape);
const auto source_polygon = autoware_utils::to_polygon2d(source_object.pose, source_object.shape);
if (boost::geometry::area(source_polygon) < MIN_AREA) return 0.0;
const auto target_polygon = autoware_utils::to_polygon2d(
target_object.kinematics.pose_with_covariance.pose, target_object.shape);
const auto target_polygon = autoware_utils::to_polygon2d(target_object.pose, target_object.shape);
if (boost::geometry::area(target_polygon) < MIN_AREA) return 0.0;
@@ -113,6 +108,4 @@ bool convertConvexHullToBoundingBox(
// calc new center
const Eigen::Vector2d center{
input_object.kinematics.pose_with_covariance.pose.position.x,
input_object.kinematics.pose_with_covariance.pose.position.y};
const auto yaw = tf2::getYaw(input_object.kinematics.pose_with_covariance.pose.orientation);
const Eigen::Vector2d center{input_object.pose.position.x, input_object.pose.position.y};
const auto yaw = tf2::getYaw(input_object.pose.orientation);
const Eigen::Matrix2d R_inv = Eigen::Rotation2Dd(-yaw).toRotationMatrix();
@@ -123,4 +116,4 @@ bool convertConvexHullToBoundingBox(
output_object = input_object;
output_object.kinematics.pose_with_covariance.pose.position.x = new_center.x();
output_object.kinematics.pose_with_covariance.pose.position.y = new_center.y();
output_object.pose.position.x = new_center.x();
output_object.pose.position.y = new_center.y();
@@ -137,3 +130,3 @@ bool getMeasurementYaw(
{
measurement_yaw = tf2::getYaw(object.kinematics.pose_with_covariance.pose.orientation);
measurement_yaw = tf2::getYaw(object.pose.orientation);
@@ -180,6 +173,11 @@ enum BBOX_IDX {
*/
int getNearestCornerOrSurface(
const double x, const double y, const double yaw, const double width, const double length,
const geometry_msgs::msg::Transform & self_transform)
void getNearestCornerOrSurface(
const geometry_msgs::msg::Transform & self_transform, types::DynamicObject & object)
{
const double x = object.pose.position.x;
const double y = object.pose.position.y;
const double yaw = tf2::getYaw(object.pose.orientation);
const double width = object.shape.dimensions.y;
const double length = object.shape.dimensions.x;
// get local vehicle pose
@@ -193,3 +191,3 @@ int getNearestCornerOrSurface(
// Determine Index
// Determine anchor point
// x+ (front)
@@ -200,104 +198,57 @@ int getNearestCornerOrSurface(
// x- (rear)
int xgrid = 0;
int ygrid = 0;
const int labels[3][3] = {
{BBOX_IDX::FRONT_L_CORNER, BBOX_IDX::FRONT_SURFACE, BBOX_IDX::FRONT_R_CORNER},
{BBOX_IDX::LEFT_SURFACE, BBOX_IDX::INSIDE, BBOX_IDX::RIGHT_SURFACE},
{BBOX_IDX::REAR_L_CORNER, BBOX_IDX::REAR_SURFACE, BBOX_IDX::REAR_R_CORNER}};
double anchor_x = 0;
double anchor_y = 0;
if (xl > length / 2.0) {
xgrid = 0; // front
anchor_x = length / 2.0;
} else if (xl > -length / 2.0) {
xgrid = 1; // middle
anchor_x = 0;
} else {
xgrid = 2; // rear
anchor_x = -length / 2.0;
}
if (yl > width / 2.0) {
ygrid = 0; // left
anchor_y = width / 2.0;
} else if (yl > -width / 2.0) {
ygrid = 1; // middle
anchor_y = 0;
} else {
ygrid = 2; // right
anchor_y = -width / 2.0;
}
return labels[xgrid][ygrid]; // 0 to 7 + 1(null) value
}
/**
* @brief Calc bounding box center offset caused by shape change
* @param dw: width update [m] = w_new - w_old
* @param dl: length update [m] = l_new - l_old
* @param indx: nearest corner index
* @return 2d offset vector caused by shape change
*/
inline Eigen::Vector2d calcOffsetVectorFromShapeChange(
const double dw, const double dl, const int indx)
{
Eigen::Vector2d offset;
// if surface
if (indx == BBOX_IDX::FRONT_SURFACE) {
offset(0, 0) = dl / 2.0; // move forward
offset(1, 0) = 0;
} else if (indx == BBOX_IDX::RIGHT_SURFACE) {
offset(0, 0) = 0;
offset(1, 0) = -dw / 2.0; // move right
} else if (indx == BBOX_IDX::REAR_SURFACE) {
offset(0, 0) = -dl / 2.0; // move backward
offset(1, 0) = 0;
} else if (indx == BBOX_IDX::LEFT_SURFACE) {
offset(0, 0) = 0;
offset(1, 0) = dw / 2.0; // move left
}
// if corner
if (indx == BBOX_IDX::FRONT_R_CORNER) {
offset(0, 0) = dl / 2.0; // move forward
offset(1, 0) = -dw / 2.0; // move right
} else if (indx == BBOX_IDX::REAR_R_CORNER) {
offset(0, 0) = -dl / 2.0; // move backward
offset(1, 0) = -dw / 2.0; // move right
} else if (indx == BBOX_IDX::REAR_L_CORNER) {
offset(0, 0) = -dl / 2.0; // move backward
offset(1, 0) = dw / 2.0; // move left
} else if (indx == BBOX_IDX::FRONT_L_CORNER) {
offset(0, 0) = dl / 2.0; // move forward
offset(1, 0) = dw / 2.0; // move left
}
return offset; // do nothing if indx == INVALID or INSIDE
object.anchor_point.x = anchor_x;
object.anchor_point.y = anchor_y;
}
/**
* @brief Convert input object center to tracking point based on nearest corner information
* 1. update anchor offset vector, 2. offset input bbox based on tracking_offset vector and
* prediction yaw angle
* @param w: last input bounding box width
* @param l: last input bounding box length
* @param indx: last input bounding box closest corner index
* @param input_object: input object bounding box
* @param yaw: current yaw estimation
* @param offset_object: output tracking measurement to feed ekf
* @return nearest corner index(int)
*/
void calcAnchorPointOffset(
const double w, const double l, const int indx, const types::DynamicObject & input_object,
const double & yaw, types::DynamicObject & offset_object, Eigen::Vector2d & tracking_offset)
const types::DynamicObject & this_object, Eigen::Vector2d & tracking_offset,
types::DynamicObject & updating_object)
{
// copy value
offset_object = input_object;
// invalid index
if (indx == BBOX_IDX::INSIDE) {
return; // do nothing
const geometry_msgs::msg::Point anchor_vector = updating_object.anchor_point;
// invalid anchor
if (anchor_vector.x <= 1e-6 && anchor_vector.y <= 1e-6) {
return;
}
double input_yaw = tf2::getYaw(updating_object.pose.orientation);
// current object width and height
const double w_n = input_object.shape.dimensions.y;
const double l_n = input_object.shape.dimensions.x;
const double length = this_object.shape.dimensions.x;
const double width = this_object.shape.dimensions.y;
// update offset
const Eigen::Vector2d offset = calcOffsetVectorFromShapeChange(w_n - w, l_n - l, indx);
tracking_offset = offset;
tracking_offset = Eigen::Vector2d(anchor_vector.x, anchor_vector.y);
if (tracking_offset.x() > 0) {
tracking_offset.x() -= length / 2.0;
} else if (tracking_offset.x() < 0) {
tracking_offset.x() += length / 2.0;
}
if (tracking_offset.y() > 0) {
tracking_offset.y() -= width / 2.0;
} else if (tracking_offset.y() < 0) {
tracking_offset.y() += width / 2.0;
}
// offset input object
const Eigen::Matrix2d R = Eigen::Rotation2Dd(yaw).toRotationMatrix();
const Eigen::Matrix2d R = Eigen::Rotation2Dd(input_yaw).toRotationMatrix();
const Eigen::Vector2d rotated_offset = R * tracking_offset;
offset_object.kinematics.pose_with_covariance.pose.position.x += rotated_offset.x();
offset_object.kinematics.pose_with_covariance.pose.position.y += rotated_offset.y();
updating_object.pose.position.x += rotated_offset.x();
updating_object.pose.position.y += rotated_offset.y();
}

RoutingAdaptor::on_timer has 1 complex conditionals with 3 branches, threshold = 2

A complex conditional is an expression inside a branch such as an if-statmeent which consists of multiple, logical operations. Example: if (x.started() && y.running()).Complex conditionals make the code even harder to read, and contribute to the Complex Method code smell. Encapsulate them.

Apply the DECOMPOSE CONDITIONAL refactoring so that the complex conditional is encapsulated in a separate function with a good name that captures the business rule. Optionally, for simple expressions, introduce a new variable which holds the result of the complex conditional.

To get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes.

SAMPLE
complex_conditional.js
@@ -1,16 +1,34 @@
function messageReceived(message, timeReceived) {
// Ignore all messages which aren't from known customers:
if (!message.sender &&
customers.getId(message.name) == null) {
// Refactoring #1: encapsulate the business rule in a
// function. A clear name replaces the need for the comment:
if (!knownCustomer(message)) {
log('spam received -- ignoring');
return;
}
// Provide an auto-reply when outside business hours:
if ((timeReceived.getHours() > 17) ||
(timeReceived.getHours() < 8)) {
// Refactoring #2: encapsulate the business rule.
// Again, note how a clear function name replaces the
// need for a code comment:
if (outsideBusinessHours(timeReceived)) {
return autoReplyTo(message);
}
pingAgentFor(message);
}
function outsideBusinessHours(timeReceived) {
// Refactoring #3: replace magic numbers with
// symbols that communicate with the code reader:
const closingHour = 17;
const openingHour = 8;
const hours = timeReceived.getHours();
// Refactoring #4: simple conditional rules can
// be further clarified by introducing a variable:
const afterClosing = hours > closingHour;
const beforeOpening = hours < openingHour;
// Yeah -- look how clear the business rule is now!
return afterClosing || beforeOpening;
}

RoutingAdaptor::on_timer has 3 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function

A Bumpy Road is a function that contains multiple chunks of nested conditional logic inside the same function. The deeper the nesting and the more bumps, the lower the code health.

A bumpy code road represents a lack of encapsulation which becomes an obstacle to comprehension. In imperative languages there’s also an increased risk for feature entanglement, which leads to complex state management. CodeScene considers the following rules for the code health impact: 1) The deeper the nested conditional logic of each bump, the higher the tax on our working memory. 2) The more bumps inside a function, the more expensive it is to refactor as each bump represents a missing abstraction. 3) The larger each bump – that is, the more lines of code it spans – the harder it is to build up a mental model of the function. The nesting depth for what is considered a bump is levels of conditionals.

Bumpy Road implementations indicate a lack of encapsulation. Check out the detailed description of the Bumpy Road code health issue.

A Bumpy Road often suggests that the function/method does too many things. The first refactoring step is to identify the different possible responsibilities of the function. Consider extracting those responsibilities into smaller, cohesive, and well-named functions. The EXTRACT FUNCTION refactoring is the primary response.