lhotari commented on code in PR #23958:
URL: https://github.com/apache/pulsar/pull/23958#discussion_r1950800506
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/cache/PendingReadsManager.java:
##########
@@ -211,73 +211,95 @@ private FindPendingReadOutcome
findPendingRead(PendingReadKey key, ConcurrentMap
private class PendingRead {
final PendingReadKey key;
final ConcurrentMap<PendingReadKey, PendingRead> ledgerCache;
- final List<ReadEntriesCallbackWithContext> callbacks = new
ArrayList<>(1);
- boolean completed = false;
+ final List<ReadEntriesCallbackWithContext> listeners = new
ArrayList<>(1);
+ PendingReadState state = PendingReadState.INITIALISED;
+
+ enum PendingReadState {
+ INITIALISED,
+ ATTACHED,
+ COMPLETED
+ }
Review Comment:
> Actually I'm wondering if we need such a enum here? Could it be replaced
by two boolean flags?
Sure there are multiple possibilities, like always with code. Multiple
boolean flags to hold state that is really a single state is usually a code
smell. An enum is very lightweight and usually results in easier to read code
since the values are self describing. Using enums is recommended over boolean
flags in good coding practices even when there are only 2 choices.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
