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

No Bytes.EMPTY object equality checks should be used #16639

Closed
tinker-michaelj opened this issue Nov 18, 2024 · 1 comment · Fixed by #17213
Closed

No Bytes.EMPTY object equality checks should be used #16639

tinker-michaelj opened this issue Nov 18, 2024 · 1 comment · Fixed by #17213
Assignees
Labels
Estimated Issues that has been groomed

Comments

@tinker-michaelj
Copy link
Collaborator

Description

A zero-length PBJ Bytes is not necessarily Bytes.EMPTY so nothing should depend on object equality with Bytes.EMPTY, especially here.

Steps to reproduce

Reconnect a node, sometimes this will give a false positive there is pending PREPARE_UPGRADE work to catch up on.

Additional context

No response

Hedera network

No response

Version

multiple

Operating system

None

@anthony-swirldslabs
Copy link
Contributor

There's a similar, although unrelated to Bytes, code just a few lines above:

https://github.com/hashgraph/hedera-services/blob/develop/hedera-node/hedera-network-admin-service-impl/src/main/java/com/hedera/node/app/service/networkadmin/impl/WritableFreezeStore.java#L69

Using == for comparing objects is an anti-pattern in Java. Introducing and (ab)using it in our code base establishes it as a "valid" pattern which engineers will keep repeating over and over, causing all sorts of bugs.

Please do replace any and all usages of == with calls to equals() when comparing any objects (and not only Bytes), unless it's a special case where a reference comparison must be performed by design, in which case it's mandatory to add a comment directly in code indicating that the usage of == in a particular line is by design and explain why.

@derektriley derektriley added the Estimated Issues that has been groomed label Nov 19, 2024
@netopyr netopyr moved this from 📋 Backlog to To Do in Services Team Nov 25, 2024
@kimbor kimbor self-assigned this Jan 4, 2025
@kimbor kimbor moved this from To Do to 👷🏼‍♀️ In Progress in Services Team Jan 4, 2025
@kimbor kimbor moved this from 👷🏼‍♀️ In Progress to 👀 In Review in Services Team Jan 4, 2025
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in Services Team Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Estimated Issues that has been groomed
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants