Skip to content

fix: Fix some issues with StringUtilBenchmark #229

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

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

Conversation

dwalluck
Copy link
Contributor

@dwalluck dwalluck commented Apr 2, 2025

  • Make Random static even though in practice it should only be created once already (fixes SpotBugs warning)
  • Initialize fields to empty to prevent warning "@NullMarked fields must be initialized"
  • Iterate over decodedData directly instead of using the DATA_COUNT constant even though in practice these should always have the same length
  • Fill data fields directly instead of using a local variables/parameters with the same names

* Make `Random` static even though in practice it should only be
  created once already (fixes SpotBugs warning)
* Initialize fields to empty to prevent warning "`@NullMarked` fields
  must be initialized"
* Iterate over `decodedData` directly instead of using the `DATA_COUNT`
  constant even though in practice these should always have the same
  length
* Fill data fields directly instead of using a local
  variables/parameters with the same names
@dwalluck dwalluck force-pushed the string-util-benchmark branch from de755c4 to 7bfb05b Compare April 2, 2025 15:28
Comment on lines 71 to 76
decodedData = createDecodedData();
encodedData = encodeData(decodedData);
createDecodedData();
createEncodedData();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a big fan of methods that need to be called in a very precise order to work. What about merging these methods?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants