DonalEvans commented on a change in pull request #6697:
URL: https://github.com/apache/geode/pull/6697#discussion_r670614514
##########
File path:
geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
##########
@@ -4,10 +4,8 @@ toData,72
org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl,6
Review comment:
This 6 needs to be a 4 now that two of the methods below have been
removed.
##########
File path:
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java
##########
@@ -824,61 +765,6 @@ public void fromDataPre_GFE_9_0_0_0(DataInput in,
DeserializationContext context
// Assert.assertTrue(getPort() > 0);
Review comment:
Can this commented-out code be removed?
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/FilterRoutingInfo.java
##########
@@ -509,22 +453,22 @@ private void deserialize() {
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
- if (this.interestedClients != null && this.interestedClients.size() > 0)
{
+ if (interestedClients != null && interestedClients.size() > 0) {
sb.append("interestedClients:");
- sb.append(this.interestedClients);
+ sb.append(interestedClients);
}
- if (this.interestedClientsInv != null &&
this.interestedClientsInv.size() > 0) {
+ if (interestedClientsInv != null && interestedClientsInv.size() > 0) {
sb.append(", interestedClientsInv:");
- sb.append(this.interestedClientsInv);
+ sb.append(interestedClientsInv);
}
if (InternalDistributedSystem.getLogger().finerEnabled()) {
Review comment:
This method is deprecated. It might be better to replace it with:
```
LogService.getLogger().isTraceEnabled()
```
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/FilterRoutingInfo.java
##########
@@ -386,121 +330,121 @@ public void toData(DataOutput out) throws IOException {
byte[] myData = StaticSerialization.getThreadLocalByteArray(size);
hdos = new HeapDataOutputStream(myData);
hdos.disallowExpansion();
- if (this.cqs == null) {
+ if (cqs == null) {
hdos.writeBoolean(false);
} else {
hdos.writeBoolean(true);
InternalDataSerializer.writeArrayLength(cqs.size(), hdos);
- for (Iterator it = this.cqs.entrySet().iterator(); it.hasNext();) {
- Map.Entry e = (Map.Entry) it.next();
+ for (final Map.Entry<Long, Integer> longIntegerEntry : cqs.entrySet())
{
// most cq IDs and all event types are small ints, so we use an
optimized
// write that serializes 7 bits at a time in a compact form
- InternalDataSerializer.writeUnsignedVL((Long) e.getKey(), hdos);
- InternalDataSerializer.writeUnsignedVL((Integer) e.getValue(), hdos);
+ InternalDataSerializer.writeUnsignedVL(longIntegerEntry.getKey(),
hdos);
+ InternalDataSerializer.writeUnsignedVL(longIntegerEntry.getValue(),
hdos);
}
}
- InternalDataSerializer.writeSetOfLongs(this.interestedClients,
this.longIDs, hdos);
- InternalDataSerializer.writeSetOfLongs(this.interestedClientsInv,
this.longIDs, hdos);
+ InternalDataSerializer.writeSetOfLongs(interestedClients, longIDs, hdos);
+ InternalDataSerializer.writeSetOfLongs(interestedClientsInv, longIDs,
hdos);
Review comment:
This code block is duplicated in `toDataPre_GFE_8_0_0_0()`. Could it be
pulled out into a method, maybe something like "writeCQData()"? If the
`toDataPre_GFE_8_0_0_0()` method is shortly to be removed as well though, then
there's no need to bother with this.
##########
File path:
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java
##########
@@ -284,7 +279,7 @@ public int compareTo(MemberIdentifier other, boolean
compareMemberData,
String myName = getName();
String otherName = other.getName();
- if (!(other.isPartial() || this.isPartial())) {
+ if (!(other.isPartial() || isPartial())) {
if (myName == null && otherName == null) {
Review comment:
The warning about an empty if statement here can be fixed by refactoring
the block to:
```
if (myName != null || otherName != null) {
if (myName == null) {
return -1;
} else if (otherName == null) {
return 1;
} else {
int i = myName.compareTo(otherName);
if (i != 0) {
return i;
}
}
}
```
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/FilterRoutingInfo.java
##########
@@ -231,16 +209,16 @@ public void fromData(DataInput in) throws IOException,
ClassNotFoundException {
InternalDataSerializer.invokeFromData(fInfo, in);
Review comment:
Further up in this method there's a call to
`GemFireCacheImpl.getInstance()`, which is deprecated. Would it be possible to
replace it with a non-deprecated call, possibly:
```
InternalDistributedSystem.getConnectedInstance().getCache();
```
--
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]
