Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR cleans up legacy code by replacing custom utility methods with standard Java alternatives and optimizing array creation across multiple modules. Key changes include:
- Refactoring null handling in MetricDefinition.java by using Objects.requireNonNullElse instead of ObjectUtils.defaultIfNull.
- Introducing empty array constants (e.g., FUTURE_EMPTY_ARRAY, EMPTY_STRING_ARRAY) to replace in-line array creation, reducing unnecessary allocations.
- Removing unused logging and deprecated utilities (e.g., ObjectUtils.java) for cleaner code.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| inspector-module/inspector-web/src/main/java/com/navercorp/pinpoint/inspector/web/definition/MetricDefinition.java | Replace custom null-check utility calls with standard Java methods. |
| commons/src/main/java/com/navercorp/pinpoint/common/util/concurrent/FutureUtils.java | Use a static empty array constant to optimize allocations in toArray calls. |
| commons/src/main/java/com/navercorp/pinpoint/common/util/ObjectUtils.java | Remove legacy utility class replaced by standard methods. |
| commons-server/src/main/java/com/navercorp/pinpoint/common/server/util/InetAddressUtils.java | Minor cleanup including improved ArrayList initialization and proper import ordering. |
| commons-hbase/src/main/java/com/navercorp/pinpoint/common/hbase/parallel/ParallelResultScanner.java | Utilize a constant empty array to reduce temporary array creation in toArray calls. |
| agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/PinpointProfilerPackageFilter.java | Remove unnecessary logging to simplify the class. |
| agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMClassNodeAdapter.java | Replace repetitive empty array creations with a static constant for better performance. |
| agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/storage/AsyncQueueingUriStatStorage.java | Improve type safety by using a pre-declared empty array in collection-to-array conversion. |
| agent-module/bootstraps/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/util/spring/StringUtils.java | Introduce a constant empty array and use token.isEmpty for clearer string checks. |
Comments suppressed due to low confidence (1)
agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/plugin/PinpointProfilerPackageFilter.java:26
- [nitpick] Removal of logging may reduce debug visibility; please confirm that this cleanup aligns with the intended logging strategy in production environments.
public class PinpointProfilerPackageFilter implements ClassNameFilter {
| this.preProcess = Objects.requireNonNullElse(preProcess, EmptyPreProcessor.INSTANCE.getName()); | ||
| this.postProcess = Objects.requireNonNullElse(postProcess, EmptyPostProcessor.INSTANCE.getName()); |
There was a problem hiding this comment.
[nitpick] Ensure that switching from ObjectUtils.defaultIfNull (which applied a toString conversion for non-null values in preProcess and postProcess) to Objects.requireNonNullElse preserves the intended behavior for these fields.
| this.preProcess = Objects.requireNonNullElse(preProcess, EmptyPreProcessor.INSTANCE.getName()); | |
| this.postProcess = Objects.requireNonNullElse(postProcess, EmptyPostProcessor.INSTANCE.getName()); | |
| this.preProcess = Objects.requireNonNullElse(preProcess != null ? preProcess.toString() : null, EmptyPreProcessor.INSTANCE.getName()); | |
| this.postProcess = Objects.requireNonNullElse(postProcess != null ? postProcess.toString() : null, EmptyPostProcessor.INSTANCE.getName()); |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12500 +/- ##
============================================
- Coverage 33.58% 33.57% -0.01%
+ Complexity 10746 10745 -1
============================================
Files 3942 3941 -1
Lines 91887 91887
Branches 9558 9558
============================================
- Hits 30859 30855 -4
- Misses 58387 58389 +2
- Partials 2641 2643 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request introduces several changes across multiple files to improve code readability, reduce redundancy, and replace deprecated or less efficient methods with modern alternatives. The most significant updates include the introduction of reusable empty arrays, the replacement of
defaultIfNullwithObjects.requireNonNullElse, and minor optimizations to array creation and logging.Codebase simplification:
Reusable empty arrays:
EMPTY_STRING_ARRAY,RESULT_EMPTY_ARRAY, andFUTURE_EMPTY_ARRAYto avoid creating new empty arrays repeatedly. These are used in methods liketoArray()across multiple files, includingStringUtils,ASMClassNodeAdapter,ParallelResultScanner, andFutureUtils. [1] [2] [3]Optimized array creation:
new String[0]and similar constructs with reusable constants likeEMPTY_STRING_ARRAYandRESULT_EMPTY_ARRAYin methods such astoArray()andgetInterfaceNames()to improve performance and readability. [1] [2] [3]Modernization of code:
Use of
Objects.requireNonNullElse:ObjectUtils.defaultIfNullwithObjects.requireNonNullElseinMetricDefinitionto leverage built-in Java functionality for handling default values.Simplified logging:
Loggerinstances and related debug checks inPinpointProfilerPackageFilterto streamline the class and eliminate unnecessary dependencies. [1] [2]Minor improvements:
Optimized collection handling:
ArrayListinstantiation with the diamond operator (<>) for cleaner code inInetAddressUtils.Removed unused utility class:
ObjectUtils, which is no longer needed due to the adoption ofObjects.requireNonNullElse.