Code optimization: convert class fields to local variables where it's…

… possible
Adobe-Consulting-Services · Mar 25, 2024 · ca4cacf · ca4cacf
1 parent f2ddea8
commit ca4cacf
Show file tree

Hide file tree

Showing 11 changed files with 41 additions and 83 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
 - #3294 - Cloud manager report issues partial fix
 - #3295 - Updated the annotations in QueryReportConfig fixing the query manager issue due to empty query language
 - #3284 - Allow anonymous to read redirect caconfig options
+- #2854 - Code optimization: convert class fields to local variables
 
 ## 6.4.0 - 2024-02-22
 

diff --git a/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java b/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java
@@ -126,8 +126,6 @@ public final class ErrorPageHandlerImpl implements ErrorPageHandlerService {
     /* Fallback Error Code Extension */
     private static final String DEFAULT_FALLBACK_ERROR_NAME = "500";
 
-    private String fallbackErrorName = DEFAULT_FALLBACK_ERROR_NAME;
-
     @Property(
             label = "Fallback error page name",
             description = "Error page name (not path) to use if a valid Error Code/Error Servlet Name cannot be "
@@ -852,7 +850,7 @@ private void configure(ComponentContext componentContext) {
                 PropertiesUtil.toString(config.get(legacyPrefix + PROP_ERROR_PAGE_EXTENSION),
                         DEFAULT_ERROR_PAGE_EXTENSION));
 
-        this.fallbackErrorName = PropertiesUtil.toString(config.get(PROP_FALLBACK_ERROR_NAME),
+        final String fallbackErrorName = PropertiesUtil.toString(config.get(PROP_FALLBACK_ERROR_NAME),
                 PropertiesUtil.toString(config.get(legacyPrefix + PROP_FALLBACK_ERROR_NAME),
                         DEFAULT_FALLBACK_ERROR_NAME));
 
@@ -936,7 +934,7 @@ private void configure(ComponentContext componentContext) {
         pw.printf("Enabled: %s", this.enabled).println();
         pw.printf("System Error Page Path: %s", this.systemErrorPagePath).println();
         pw.printf("Error Page Extension: %s", this.errorPageExtension).println();
-        pw.printf("Fallback Error Page Name: %s", this.fallbackErrorName).println();
+        pw.printf("Fallback Error Page Name: %s", fallbackErrorName).println();
 
         pw.printf("Resource Not Found - Behavior: %s", this.notFoundBehavior).println();
         pw.printf("Resource Not Found - Exclusion Path Patterns %s", Arrays.toString(tmpNotFoundExclusionPatterns)).println();

diff --git a/bundle/src/main/java/com/adobe/acs/commons/httpcache/config/impl/HttpCacheConfigImpl.java b/bundle/src/main/java/com/adobe/acs/commons/httpcache/config/impl/HttpCacheConfigImpl.java
@@ -108,7 +108,6 @@ public class HttpCacheConfigImpl implements HttpCacheConfig {
               cardinality = Integer.MAX_VALUE)
     static final String PROP_BLACKLISTED_REQUEST_URI_PATTERNS =
             "httpcache.config.requesturi.patterns.blacklisted";
-    private List<String> blacklistedRequestUriPatterns;
     private List<Pattern> blacklistedRequestUriPatternsAsRegEx = Collections.emptyList();
 
     // Authentication requirement
@@ -136,7 +135,6 @@ public class HttpCacheConfigImpl implements HttpCacheConfig {
                       + ". This accepts " + "REGEX. Example - /etc/my-products(.*)",
               cardinality = Integer.MAX_VALUE)
     static final String PROP_CACHE_INVALIDATION_PATH_PATTERNS = "httpcache.config.invalidation.oak.paths";
-    private List<String> cacheInvalidationPathPatterns;
     private List<Pattern> cacheInvalidationPathPatternsAsRegEx = Collections.emptyList();
 
     // Cache store
@@ -270,7 +268,7 @@ protected void activate(Map<String, Object> configs) {
         excludedCookieKeys = Arrays.asList(PropertiesUtil.toStringArray(configs.get(PROP_RESPONSE_COOKIE_KEY_EXCLUSIONS), new String[]{}));
 
         // Request URIs - Blacklisted.
-        blacklistedRequestUriPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs
+        List<String> blacklistedRequestUriPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs
                 .get(PROP_BLACKLISTED_REQUEST_URI_PATTERNS), new String[]{}));
         blacklistedRequestUriPatternsAsRegEx = compileToPatterns(blacklistedRequestUriPatterns);
 
@@ -287,7 +285,7 @@ protected void activate(Map<String, Object> configs) {
         expiryOnUpdate = PropertiesUtil.toLong(configs.get(PROP_EXPIRY_ON_UPDATE), DEFAULT_EXPIRY_ON_UPDATE);
 
         // Cache invalidation paths.
-        cacheInvalidationPathPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs
+        List<String> cacheInvalidationPathPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs
                 .get(PROP_CACHE_INVALIDATION_PATH_PATTERNS), new String[]{}));
         cacheInvalidationPathPatternsAsRegEx = compileToPatterns(cacheInvalidationPathPatterns);
 

diff --git a/...e/src/main/java/com/adobe/acs/commons/httpcache/store/mem/impl/MemHttpCacheStoreImpl.java b/...e/src/main/java/com/adobe/acs/commons/httpcache/store/mem/impl/MemHttpCacheStoreImpl.java
@@ -94,16 +94,14 @@ public class MemHttpCacheStoreImpl extends AbstractGuavaCacheMBean<CacheKey, Mem
     private static final String PROP_MAX_SIZE_IN_MB = "httpcache.cachestore.memcache.maxsize";
     private static final long DEFAULT_MAX_SIZE_IN_MB = 10L; // Defaults to 10MB.
 
-    private long maxSizeInMb;
-
     /** Cache - Uses Google Guava's cache */
     private Cache<CacheKey, MemCachePersistenceObject> cache;
 
     @Activate
     protected void activate(Map<String, Object> configs) {
         // Read config and populate values.
         ttl = PropertiesUtil.toLong(configs.get(PROP_TTL), DEFAULT_TTL);
-        maxSizeInMb = PropertiesUtil.toLong(configs.get(PROP_MAX_SIZE_IN_MB), DEFAULT_MAX_SIZE_IN_MB);
+        long maxSizeInMb = PropertiesUtil.toLong(configs.get(PROP_MAX_SIZE_IN_MB), DEFAULT_MAX_SIZE_IN_MB);
 
         // Initializing the cache.
         // If cache is present, invalidate all and reinitailize the cache.

diff --git a/bundle/src/main/java/com/adobe/acs/commons/reports/models/ReportRunner.java b/bundle/src/main/java/com/adobe/acs/commons/reports/models/ReportRunner.java
@@ -45,8 +45,6 @@ public class ReportRunner {
 
   private String failureMessage;
 
-  private int page;
-
   private ReportExecutor reportExecutor;
 
   private SlingHttpServletRequest request;
@@ -71,15 +69,15 @@ public ReportRunner(SlingHttpServletRequest request) {
   }
 
   @SuppressWarnings("squid:S2658") // class name is from a trusted source
-  private boolean executeConfig(Resource config, SlingHttpServletRequest request) {
+  private boolean executeConfig(Resource config, SlingHttpServletRequest request, int page) {
     log.trace("executeConfig");
     try {
       Class<?> exClass = ReportExecutorProvider.INSTANCE.getReportExecutor(dynamicClassLoaderManager, config);
       Object model = request.adaptTo(exClass);
       if (model instanceof ReportExecutor) {
         ReportExecutor ex = (ReportExecutor) model;
         ex.setConfiguration(config);
-        ex.setPage(this.page);
+        ex.setPage(page);
         this.reportExecutor = ex;
         return true;
       } else {
@@ -110,6 +108,7 @@ public ReportExecutor getReportExecutor() {
   protected void init() {
     log.trace("init");
 
+    int page;
     try {
       page = Integer.parseInt(request.getParameter("page"), 10);
     } catch (Exception e) {
@@ -123,7 +122,7 @@ protected void init() {
       Iterator<Resource> children = configCtr.listChildren();
       while (children.hasNext()) {
         Resource config = children.next();
-        if (executeConfig(config, request)) {
+        if (executeConfig(config, request, page)) {
           log.debug("Successfully executed report with configuration: {}", config);
           resultsRetrieved = true;
           break;

diff --git a/bundle/src/main/java/com/adobe/acs/commons/twitter/impl/TwitterAdapterFactory.java b/bundle/src/main/java/com/adobe/acs/commons/twitter/impl/TwitterAdapterFactory.java
@@ -41,10 +41,9 @@
 class TwitterAdapterFactory implements AdapterFactory {
 
     TwitterAdapterFactory(String httpProxyHost, int httpProxyPort, boolean useSsl) {
-        this.httpProxyHost = httpProxyHost;
-        this.httpProxyPort = httpProxyPort;
-        this.useSsl = useSsl;
-        this.factory = new TwitterFactory(buildConfiguration());
+        this.factory = new TwitterFactory(
+                buildConfiguration(httpProxyHost, httpProxyPort, useSsl)
+        );
     }
 
     @VisibleForTesting
@@ -58,13 +57,6 @@ class TwitterAdapterFactory implements AdapterFactory {
 
     private final TwitterFactory factory;
 
-    private final String httpProxyHost;
-
-    private final int httpProxyPort;
-
-    private final boolean useSsl;
-
-
 
     @SuppressWarnings("unchecked")
     @Override
@@ -87,7 +79,7 @@ public <AdapterType> AdapterType getAdapter(Object adaptable, Class<AdapterType>
         return null;
     }
 
-    private Configuration buildConfiguration() {
+    private Configuration buildConfiguration(String httpProxyHost, int httpProxyPort, boolean useSsl) {
         final ConfigurationBuilder builder = new ConfigurationBuilder();
         builder.setUseSSL(useSsl);
         builder.setJSONStoreEnabled(true);

diff --git a/bundle/src/main/java/com/adobe/acs/commons/version/impl/CurrentEvolutionImpl.java b/bundle/src/main/java/com/adobe/acs/commons/version/impl/CurrentEvolutionImpl.java
@@ -40,15 +40,11 @@ public class CurrentEvolutionImpl implements Evolution {
 
     private static final Logger log = LoggerFactory.getLogger(CurrentEvolutionImpl.class);
 
-    private final Resource resource;
     private final List<EvolutionEntry> versionEntries = new ArrayList<EvolutionEntry>();
-    private EvolutionConfig config;
 
     public CurrentEvolutionImpl(Resource resource, EvolutionConfig config) {
-        this.resource = resource;
-        this.config = config;
         try {
-            populate(this.resource, 0);
+            populate(resource, config, 0);
         } catch (RepositoryException e) {
             log.warn("Could not populate Evolution", e);
         }
@@ -74,7 +70,7 @@ public List<EvolutionEntry> getVersionEntries() {
         return Collections.unmodifiableList(this.versionEntries);
     }
 
-    private void populate(Resource r, int depth) throws PathNotFoundException, RepositoryException {
+    private void populate(Resource r, EvolutionConfig config, int depth) throws PathNotFoundException, RepositoryException {
         ValueMap map = r.getValueMap();
         List<String> keys = new ArrayList<String>(map.keySet());
         Collections.sort(keys);
@@ -92,7 +88,7 @@ private void populate(Resource r, int depth) throws PathNotFoundException, Repos
             String relPath = EvolutionPathUtil.getLastRelativeResourceName(child.getPath());
             if (config.handleResource(relPath)) {
                 versionEntries.add(new CurrentEvolutionEntryImpl(child, config));
-                populate(child, depth);
+                populate(child, config, depth);
             }
             depth--;
         }

diff --git a/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionAnalyserImpl.java b/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionAnalyserImpl.java
@@ -47,8 +47,6 @@ public class EvolutionAnalyserImpl implements EvolutionAnalyser {
 
     protected static final String PROPERTY_IGNORES = "properties.ignore";
     protected static final String RESOURCE_IGNORES = "resources.ignore";
-    private String[] propertyIgnores;
-    private String[] resourceIgnores;
 
     private EvolutionConfig evolutionConfig;
 
@@ -59,8 +57,8 @@ public EvolutionContext getEvolutionContext(Resource resource) {
 
     @Activate
     protected void activate(final Map<String, String> config) {
-        propertyIgnores = PropertiesUtil.toStringArray(config.get(PROPERTY_IGNORES), new String[] { "" });
-        resourceIgnores = PropertiesUtil.toStringArray(config.get(RESOURCE_IGNORES), new String[] { "" });
+        String[] propertyIgnores = PropertiesUtil.toStringArray(config.get(PROPERTY_IGNORES), new String[]{""});
+        String[] resourceIgnores = PropertiesUtil.toStringArray(config.get(RESOURCE_IGNORES), new String[]{""});
         evolutionConfig = new EvolutionConfig(propertyIgnores, resourceIgnores);
         log.debug("Ignored properties: {}", (Object[]) propertyIgnores);
         log.debug("Ignored resources: {}", (Object[]) resourceIgnores);

diff --git a/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionContextImpl.java b/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionContextImpl.java
@@ -37,18 +37,14 @@
 public class EvolutionContextImpl implements EvolutionContext {
     private static final Logger log = LoggerFactory.getLogger(EvolutionContext.class);
 
-    private Resource resource = null;
-    private VersionHistory history = null;
-    private ResourceResolver resolver = null;
-    private VersionManager versionManager = null;
     private List<Evolution> versions = new ArrayList<Evolution>();
     private List<Evolution> evolutionItems = new ArrayList<Evolution>();
-    private EvolutionConfig config;
 
-    public EvolutionContextImpl(Resource resource, EvolutionConfig config) {
-        this.resource = resource.isResourceType("cq:Page") ? resource.getChild("jcr:content") : resource;
-        this.config = config;
-        populateEvolutions();
+    public EvolutionContextImpl(Resource providedResource, EvolutionConfig config) {
+        Resource resource = providedResource.isResourceType("cq:Page")
+                ? providedResource.getChild("jcr:content")
+                : providedResource;
+        populateEvolutions(resource, config);
     }
 
     @Override
@@ -61,11 +57,11 @@ public List<Evolution> getVersions() {
         return Collections.unmodifiableList(versions);
     }
 
-    private void populateEvolutions() {
+    private void populateEvolutions(Resource resource, EvolutionConfig config) {
         try {
-            this.resolver = resource.getResourceResolver();
-            this.versionManager = resolver.adaptTo(Session.class).getWorkspace().getVersionManager();
-            this.history = versionManager.getVersionHistory(resource.getPath());
+            ResourceResolver resolver = resource.getResourceResolver();
+            VersionManager versionManager = resolver.adaptTo(Session.class).getWorkspace().getVersionManager();
+            VersionHistory history = versionManager.getVersionHistory(resource.getPath());
             Iterator<Version> iter = history.getAllVersions();
             while (iter.hasNext()) {
                 Version next = iter.next();
@@ -80,7 +76,6 @@ private void populateEvolutions() {
             log.error("Could not find versions", e);
         }
         evolutionItems = new ArrayList<>(versions);
-        evolutionItems.add(new CurrentEvolutionImpl(this.resource, this.config));
+        evolutionItems.add(new CurrentEvolutionImpl(resource, config));
     }
-
 }
diff --git a/bundle/src/main/java/com/adobe/acs/commons/wcm/comparisons/impl/lines/LinesGenerator.java b/bundle/src/main/java/com/adobe/acs/commons/wcm/comparisons/impl/lines/LinesGenerator.java
@@ -45,10 +45,8 @@ public class LinesGenerator<T> {
     private Stepper<T> right;
 
     private T leftValue;
-    private int leftSpacer;
 
     private T rightValue;
-    private int rightSpacer;
 
     public LinesGenerator(Function<T, Serializable> toId) {
         this.toId = toId;
@@ -64,24 +62,23 @@ public List<Line<T>> generate(final Iterable<T> left, Iterable<T> right) {
         this.rightValue = this.right.next();
 
         do {
-            this.leftSpacer = this.right.positionOfIdAfterCurrent(leftValue);
-            this.rightSpacer = this.left.positionOfIdAfterCurrent(rightValue);
+            int leftSpacer = this.right.positionOfIdAfterCurrent(leftValue);
+            int rightSpacer = this.left.positionOfIdAfterCurrent(rightValue);
 
             if (leftValue != null && rightValue != null && toId.apply(leftValue).equals(toId.apply(rightValue))) {
                 addPair(lines);
 
             } else if (leftSpacer < rightSpacer && leftSpacer > 0) {
-                addWithLeftSpacers(lines);
+                addWithLeftSpacers(lines, leftSpacer);
 
             } else if (rightSpacer > 0) {
-                addWithRightSpacers(lines);
+                addWithRightSpacers(lines, rightSpacer);
 
             } else if (leftSpacer > 0) {
-                addWithLeftSpacers(lines);
+                addWithLeftSpacers(lines, leftSpacer);
 
             } else {
                 addSeperated(lines);
-
             }
         } while (leftValue != null || rightValue != null);
 
@@ -99,14 +96,14 @@ private void addSeperated(List<Line<T>> lines) {
         }
     }
 
-    private void addWithLeftSpacers(List<Line<T>> lines) {
+    private void addWithLeftSpacers(List<Line<T>> lines, int leftSpacer) {
         for (int i = 0; i < leftSpacer; i++) {
             lines.add(LineImpl.right(rightValue));
             rightValue = this.right.next();
         }
     }
 
-    private void addWithRightSpacers(List<Line<T>> lines) {
+    private void addWithRightSpacers(List<Line<T>> lines, int rightSpacer) {
         for (int i = 0; i < rightSpacer; i++) {
             lines.add(LineImpl.left(leftValue));
             leftValue = this.left.next();
@@ -118,6 +115,4 @@ private void addPair(List<Line<T>> lines) {
         this.leftValue = this.left.next();
         this.rightValue = this.right.next();
     }
-
-
 }