Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/concealer 1 #1488

Open
wants to merge 44 commits into
base: master
Choose a base branch
from
Open

Refactor/concealer 1 #1488

wants to merge 44 commits into from

Conversation

yamacir-kit
Copy link
Collaborator

@yamacir-kit yamacir-kit commented Dec 13, 2024

Description

Abstract

The package concealer code was refactored.

Background

The code of the package concealer has been modified without any refactoring since its creation until today. This is not due to technical reasons, but simply because there was no opportunity. However, now that the merge of #1454 has been completed, I thought it was a good opportunity to refactor it.

Details

As the title suggests, I plan to refactor concealer in several stages. The purpose of this pull request is to first make the code more compact without making any functional changes.

Base Class Autoware

When I designed concealer, I was struggling with the need to support three different Autoware flavors in scenario_simulator_v2: Autoware.AI, Autoware.T4B, and AutowareArchitectureProposal. The structure of the base class Autoware and its derived class AutowareUniverse is based on the assumption that Autoware with an architecture different from Autoware.Universe will appear. However, in reality, no such different type of Autoware has appeared since the emergence of Autoware.Universe. This could be interpreted as just a coincidence that it has not appeared until now, but it is also true that the code is bulky due to this structure, so for the time being, I deleted the base class and simplified the code.

Reducing Member Functions

Member functions that are only used by certain member functions have been changed to function local functions. Making them private member functions was also an option, but we decided not to do so. Based on our experience in maintenance over the past few years, the cost of understanding where a certain member function is being called from is a burden both in terms of code modification and code review. This is just a matter of preference. I don't mind changing function-local functions to private member functions at some point, so I'm doing it this way for now to make refactoring following this pull request easier.

References

None.

Destructive Changes

None.

Known Limitations

None.

Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
…irtual

Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
@yamacir-kit yamacir-kit added refactor bump patch If this pull request merged, bump patch version of the scenario_simulator_v2 labels Dec 13, 2024
@yamacir-kit yamacir-kit self-assigned this Dec 13, 2024
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
…virtual

Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
…rtual

Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
….cpp`

Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
…idation`

Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
…earSign`

Signed-off-by: yamacir-kit <httperror@404-notfound.jp>
@yamacir-kit yamacir-kit requested a review from HansRobo December 18, 2024 09:22
@yamacir-kit yamacir-kit marked this pull request as ready for review December 18, 2024 09:23
Copy link

Please retry analysis of this Pull-Request directly on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch If this pull request merged, bump patch version of the scenario_simulator_v2 refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant