readBytes() side effects
- Dominant language
- Java
- Stars
- 44
- Forks
- 15
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 12
Description
### Problem
There's a number of methods that accept a ByteBuffer or a similar object (BufferedData etc.) as input which then gets populated with whatever data the method owner object has. For example, consider:
RandomAccessSequenceAdapter.readBytes(@NonNull final ByteBuffer dst)
If the length of the data to be written is zero, then the method could be a no-op and return early. However, this method has a side effect of setting the limit on the `dst` buffer to account for the bytes written.
Even if the length turns out to be zero, the existing code (e.g. many unit tests) expects the method to update the limit of the destination buffer. This means that the method must perform more expensive computations before it can return even when no actual work needs to be done.
This side-effect is undocumented, unclear, and seems undesirable. The caller (aka the owner of the `dst` buffer) should make decisions about setting the limit. This task shouldn't be delegated to methods that merely write into the buffer.
Also, see https://github.com/hashgraph/pbj/pull/190#discussion_r1474879156
### Solution
* Find all places, including production code (in addition to the unit tests mentioned above) that rely on this side-effect
** Note that many similar methods need to be checked in many classes that implement the interfaces.
* Find a solution to avoid relying on the side-effect and update the code accordingly
* Remove the side-effect
### Alternatives
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.