[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7103: GEODE-9851: Make InterestType and DataPolicy enums.

[GitBox <gi...@apache.org>]

pivotal-jbarrett commented on a change in pull request #7103:
URL: https://github.com/apache/geode/pull/7103#discussion_r761401954



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java
##########
@@ -3874,16 +3877,17 @@ private void registerInterestKeys(Map<String, ?> regionKeys, boolean allKey, Loc
             regionsWithInterest.add(regionName);
             if (allKey) {
               ccn.registerClientInterest(regionName, e.getValue(), proxy.getProxyID(), interestType,
-                  isDurable, updatesAsInvalidates, manageEmptyRegions, 0, false);
+                  isDurable, updatesAsInvalidates, manageEmptyRegions, DataPolicy.EMPTY, false);
             } else if (InterestType.REGULAR_EXPRESSION == interestType) {
               for (final Object o : asSet(e.getValue())) {
                 ccn.registerClientInterest(regionName, o, proxy.getProxyID(),
-                    interestType, isDurable, updatesAsInvalidates, manageEmptyRegions, 0, false);
+                    interestType, isDurable, updatesAsInvalidates, manageEmptyRegions,
+                    DataPolicy.EMPTY, false);
               }
             } else {
               ccn.registerClientInterest(regionName, new ArrayList<>(asSet(e.getValue())),
                   proxy.getProxyID(), isDurable, updatesAsInvalidates, manageEmptyRegions,
-                  interestType, false);
+                  DataPolicy.EMPTY, false);

Review comment:
       @DonalEvans winner winner chicken dinner! You found the bug that validates the need for the PR and the advocation for strong types, like enums, over weaker types, like int. If you look at this method, `registerClientInterest` has overrides that take parameters in slightly different orders but also takes an `int`. In one this `int` was the `InterestType` ordinal and the other it was expected to be, and is later converted to, `DataPolicy` ordinal. You will notice that the compiler was just fine with the bug that that `interestType` was passed for the `dataPolicy` since both were `int`. By converting the parameters to strong types we find this bug at compile time.




-- 
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: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org