Delta analysis
Reviewed on Mar 15, 06:16
AvoidanceModule::plan already has high cyclomatic complexity, and now it increases in Lines of Code from 104 to 106
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:
@@ -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(); |
} |
TurnSignalDecider::getBehaviorTurnSignalInfo has a cyclomatic complexity of 23, 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:
@@ -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(); |
} |
The lines of code increases from 1192 to 1194, improve code health by reducing it to 1000
This module has 1194 lines of code (comments stripped away). This puts the module at risk of evolving into a Brain Class. Brain Classes are problematic since changes become more complex over time, harder to test, and challenging to refactor. Act now to prevent future maintenance issues.
Look for opportunities to modularize the design. This is done by identifying groups of functions that represent different responsibilities and/or operate on different data. Once you have identified the different responsibilities, then use refactorings like EXTRACT CLASS.
The mean cyclomatic complexity increases from 7.25 to 8.46, threshold = 4
Overall Code Complexity is measured by the mean cyclomatic complexity across all functions in the file. The lower the number, the better.
Cyclomatic complexity is a function level metric that measures the number of logical branches (if-else, loops, etc.). Cyclomatic complexity is a rough complexity measure, but useful as a way of estimating the minimum number of unit tests you would need. As such, prefer functions with low cyclomatic complexity (2-3 branches).
You address the overall cyclomatic complexity by a) modularizing the code, and b) abstract away the complexity. Let's look at some examples:
Modularizing the Code: Do an X-Ray and inspect the local hotspots. Are there any complex conditional expressions? If yes, then do a DECOMPOSE CONDITIONAL refactoring. Extract the conditional logic into a separate function and put a good name on that function. This clarifies the intent and makes the original function easier to read. Repeat until all complex conditional expressions have been simplified.
The lines of code increases from 1653 to 1657, improve code health by reducing it to 1000
This module has 1657 lines of code (comments stripped away). This puts the module at risk of evolving into a Brain Class. Brain Classes are problematic since changes become more complex over time, harder to test, and challenging to refactor. Act now to prevent future maintenance issues.
Look for opportunities to modularize the design. This is done by identifying groups of functions that represent different responsibilities and/or operate on different data. Once you have identified the different responsibilities, then use refactorings like EXTRACT CLASS.
The lines of code increases from 1717 to 1738, improve code health by reducing it to 1000
This module has 1738 lines of code (comments stripped away). This puts the module at risk of evolving into a Brain Class. Brain Classes are problematic since changes become more complex over time, harder to test, and challenging to refactor. Act now to prevent future maintenance issues.
Look for opportunities to modularize the design. This is done by identifying groups of functions that represent different responsibilities and/or operate on different data. Once you have identified the different responsibilities, then use refactorings like EXTRACT CLASS.
The mean cyclomatic complexity increases from 5.06 to 5.09, threshold = 4
Overall Code Complexity is measured by the mean cyclomatic complexity across all functions in the file. The lower the number, the better.
Cyclomatic complexity is a function level metric that measures the number of logical branches (if-else, loops, etc.). Cyclomatic complexity is a rough complexity measure, but useful as a way of estimating the minimum number of unit tests you would need. As such, prefer functions with low cyclomatic complexity (2-3 branches).
You address the overall cyclomatic complexity by a) modularizing the code, and b) abstract away the complexity. Let's look at some examples:
Modularizing the Code: Do an X-Ray and inspect the local hotspots. Are there any complex conditional expressions? If yes, then do a DECOMPOSE CONDITIONAL refactoring. Extract the conditional logic into a separate function and put a good name on that function. This clarifies the intent and makes the original function easier to read. Repeat until all complex conditional expressions have been simplified.
The lines of code decreases from 1895 to 1711, improve code health by reducing it to 1000
existShiftSideLane is no longer above the threshold for cyclomatic complexity
calcTurnSignalInfo is no longer above the threshold for cyclomatic complexity
existShiftSideLane is no longer above the threshold for logical blocks with deeply nested code
The mean cyclomatic complexity decreases from 4.96 to 4.76, threshold = 4
existShiftSideLane is no longer above the threshold for number of arguments
isNearEndOfShift is no longer above the threshold for number of arguments
calcTurnSignalInfo is no longer above the threshold for number of arguments
The number of functions in this module is no longer above the threshold
The lines of code decreases from 1369 to 1340, improve code health by reducing it to 1000
StartPlannerModule::calcTurnSignalInfo is no longer above the threshold for cyclomatic complexity
StartPlannerModule::calcTurnSignalInfo no longer has a complex conditional
StartPlannerModule::calcTurnSignalInfo is no longer above the threshold for logical blocks with deeply nested code