refactor: improve code quality by enforcing final on utility classes and extracting complex methods#3049
refactor: improve code quality by enforcing final on utility classes and extracting complex methods#3049nafisa1602 wants to merge 18 commits into
Conversation
…rivate constructor)
… private constructor)
…ters in GsonBuilder
… remove redundant trimToSize()
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Marcono1234
left a comment
There was a problem hiding this comment.
Thanks!
The changes look mostly good to me. I have added two comments, feel free to consider them only as suggestions.
The direct project members might have additional comments.
| jsonAdapterFactory, | ||
| newImmutableList(reflectionFilters))); | ||
|
|
||
| factories.trimToSize(); |
There was a problem hiding this comment.
Maybe it would be worth discussing whether this trimToSize() here is really needed or is premature optimization, but since the purpose of your PR is only to use "interface types over implementation types", I think this functionality change should be reverted for this PR. That is, please change the local variable back to ArrayList and add back the trimToSize() call.
Generally "interface types over implementation types" is a good principle, but for internal code it is bit less relevant, even more so if this only applies to a local variable and the specific implementation type is needed because the needed functionality does not exist for the interface type (as it is the case here).
| build | ||
|
|
||
| .DS_Store | ||
| apache-maven-3.9.6-bin.tar.gz |
There was a problem hiding this comment.
Can you please revert this? It seems to be specific to your development setup and is probably not useful for most other contributors.
…List with trimToSize, remove maven tarball from gitignore
|
Thanks for the review! I've reverted the createFactories change back to
ArrayList with trimToSize() and removed the maven tarball entry from
.gitignore. Let me know if anything else needs adjusting.
…On Mon, Jun 29, 2026 at 6:21 PM Marcono1234 ***@***.***> wrote:
***@***.**** commented on this pull request.
Thanks!
The changes look mostly good to me. I have added two comments, feel free
to consider them only as suggestions.
The direct project members might have additional comments.
------------------------------
In gson/src/main/java/com/google/gson/GsonBuilder.java
<#3049 (comment)>:
> @@ -997,7 +998,6 @@ List<TypeAdapterFactory> createFactories(
jsonAdapterFactory,
newImmutableList(reflectionFilters)));
- factories.trimToSize();
Maybe it would be worth discussing whether this trimToSize() here is
really needed or is premature optimization, but since the purpose of your
PR is only to use "interface types over implementation types", I think this
functionality change should be reverted for this PR. That is, please change
the local variable back to ArrayList and add back the trimToSize() call.
Generally "interface types over implementation types" is a good principle,
but for internal code it is bit less relevant, even more so if this only
applies to a local variable and the specific implementation type is needed
because the needed functionality does not exist for the interface type (as
it is the case here).
------------------------------
In .gitignore
<#3049 (comment)>:
> @@ -18,3 +18,4 @@ local.properties
build
.DS_Store
+apache-maven-3.9.6-bin.tar.gz
Can you please revert this? It seems to be specific to your development
setup and is probably not useful for most other contributors.
—
Reply to this email directly, view it on GitHub
<#3049?email_source=notifications&email_token=BOI67X7A3TIDFOLSXU35XYD5CJNM7A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINJZGE3TENRUGUYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4591726450>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BOI67X6GUHGPJL3JYTSCN3D5CJNM7AVCNFSNUABEKJSXA33TNF2G64TZHMZTENJTHA4DOMJ3JFZXG5LFHM2DONRUG44DONZVGOQXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/BOI67X7OH3F5L6JPPKFWBDT5CJNM7A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINJZGE3TENRUGUYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/BOI67X2KMEYS3UPDNH5AOB35CJNM7A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINJZGE3TENRUGUYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
What issues were addressed
PreJava9DateFormatProvider, ReflectionAccessFilterHelper, TroubleshootingGuide,
FormattingStyle, ISO8601Utils, and SqlTimestampTypeAdapter final
into private named methods in Excluder
Why the changes improve the system
design intent clearly to future maintainers
near 0 and make each exclusion rule independently readable and testable
Refactoring approach
Extract Method and programming to interfaces patterns, guided by PMD static analysis.
Functionality unchanged
All 4603 existing tests pass with 0 failures after every change.