-
Notifications
You must be signed in to change notification settings - Fork 0
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
Modified PNReconnectionPolicy #297
Conversation
Default and max value for maximumReconnectionRetries is 10 Added random value 0,001-2,999s to delay between retry both for Linear and Exponential policy.
src/main/java/com/pubnub/api/managers/DelayedReconnectionManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/pubnub/api/managers/DelayedReconnectionManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/pubnub/api/managers/DelayedReconnectionManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/pubnub/api/managers/DelayedReconnectionManager.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
int getNextInterval() { | ||
int timerInterval = LINEAR_INTERVAL; | ||
private boolean maxConnectionIsNotSetToInfinite() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having negation (Not
) in the middle of the name here is confusing,
how about isInfiniteReconnections
? (and flip the if
to ==
, and fix call sites)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then condition would change to:
if (!maxConnectionIsSetToInfinite() && failedCalls >= maxConnectionRetries) {
For me it is more clear to have
if (maxConnectionIsNotSetToInfinite() && failedCalls >= maxConnectionRetries) {
than
if (!maxConnectionIsSetToInfinite() && failedCalls >= maxConnectionRetries) {
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would always try to use affirmative names for boolean values (names and functions) where possible
!isSomething
or even !(isSomething)
for clarity is perfectly fine
with negative names there is a possibility to get double negatives which is really confusing, such as !isNotSomething()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me maxConnectionIsNotSetToInfinite
reads better. I created this function to describe this mysterious condition:
maxConnectionRetries != -1
I assumed the function will not be reused. In situation !isNotSomething()
is needed I would refactor it.
Anyway I will change it to maxConnectionIsSetToInfinite
@pubnub-release-bot release as v6.4.5 |
🚀 Release successfully completed 🚀 |
fix: Added reading message type from fetch messages response.
fix: Added random value 0.001-0.999s to delay between retries both for Linear and Exponential reconnection policies.