Uploaded image for project: 'Sling'
  1. Sling
  2. SLING-9973

DefaultAclManager.addAclStatement: redundant check for null

    XMLWordPrintableJSON

Details

    Description

      method on https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L242 starts as follows:

      private static void addAclStatement(Formatter formatter, String systemUser, List<AccessControlEntry> authorizations) {
              if (authorizations == null || areEmpty(authorizations)) {
                  return;
              }
              [...]
      

      utility method areEmpty:

      private static boolean areEmpty(List<AccessControlEntry> authorizations) {
              return authorizations == null || authorizations.isEmpty();
          }
      

      the extra check for null in addAclStatement can therefore be omitted.... but i suspect one got tricked by the method name.... IMHO it would be better to rename the utility method to isNullOrEmpty or areNullOrEmpty (if one considers a list being a plural object).

      Attachments

        Issue Links

          Activity

            People

              karlpauls Karl Pauls
              angela Angela Schreiber
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: