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.
marcin-cebo
requested review from
kleewho,
parfeon and
wkal-pubnub
as code owners
There was a problem hiding this comment.
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.
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.
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.
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 |
3 participants
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.