Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code optimization suggestions #2854

Open
3 tasks
noobyu6 opened this issue Jun 2, 2022 · 0 comments
Open
3 tasks

Code optimization suggestions #2854

noobyu6 opened this issue Jun 2, 2022 · 0 comments

Comments

@noobyu6
Copy link

noobyu6 commented Jun 2, 2022

Required Information

  • AEM Version, including Service Packs, Cumulative Fix Packs, etc: _______
  • ACS AEM Commons Version: master
  • Reproducible on Latest? yes/no

Hi,

I find that the private field resourceIgnores at Line 53 in the file 'acs-aem-commons/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionAnalyserImpl.java' on the master branch is only assigned and used in the method activate. Therefore, this field can be removed from the class, and become a local variable in the method activate. This transformation will normally reduce memory usage and improve readability of your code.

private String[] resourceIgnores; // line 53 this field can be replaced by local variable

protected void activate(final Map<String, String> config) {
  propertyIgnores = PropertiesUtil.toStringArray(config.get(PROPERTY_IGNORES), new String[] { "" });
  // String[] resourceIgnores = PropertiesUtil.toStringArray(config.get(RESOURCE_IGNORES), new 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);
    }

Besides, there are other fields like this.I sorted them out and put them in the table below.

Location Field
com.adobe.acs.commons.version.impl.EvolutionAnalyserImpl propertyIgnores
com.adobe.acs.commons.version.impl.EvolutionContextImpl history
com.adobe.acs.commons.version.impl.EvolutionContextImpl resolver
com.adobe.acs.commons.version.impl.EvolutionContextImpl versionManager
com.adobe.acs.commons.wcm.impl.AemEnvironmentIndicatorFilter color
com.adobe.acs.commons.wcm.impl.AemEnvironmentIndicatorFilter cssOverride
com.adobe.acs.commons.fam.Failure error
com.adobe.acs.commons.fam.Failure stackTrace
com.adobe.acs.commons.errorpagehandler.impl.ErrorPageHandlerImpl fallbackErrorName
com.adobe.acs.commons.httpcache.config.impl.HttpCacheConfigImpl cacheInvalidationPathPatterns
com.adobe.acs.commons.httpcache.config.impl.HttpCacheConfigImpl blacklistedRequestUriPatterns
com.adobe.acs.commons.httpcache.store.mem.impl.MemHttpCacheStoreImpl maxSizeInMb

In addition, the private field resource at Line 42 in the file 'acs-aem-commons/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionContextImpl.java ' on the master branch is only assigned and used in the method EvolutionContextImpl. Therefore, this field can be removed from the class, and become a local variable in the method EvolutionContextImpl. Besides, for method populateEvolutions, the field resource can be converted to an input parameter.This transformation will normally reduce memory usage and improve readability of your code.I will be happy if this transformation is helpful.

private Resource resource = null;// line 42 this field can be replaced by local variable

public EvolutionContextImpl(Resource resource, EvolutionConfig config) {
  // Resource resource = resource.isResourceType("cq:Page") ? resource.getChild("jcr:content") : resource;
	this.resource = resource.isResourceType("cq:Page") ? resource.getChild("jcr:content") : resource;
	this.config = config;
  // populateEvolutions(resource);
	populateEvolutions();
}
private void populateEvolutions(Resource resource) {
	try {
			this.resolver = resource.getResourceResolver();
			this.versionManager = resolver.adaptTo(Session.class).getWorkspace().getVersionManager();
			this.history = versionManager.getVersionHistory(resource.getPath());
			Iterator<Version> iter = history.getAllVersions();
			while (iter.hasNext()) {
      		Version next = iter.next();
          String versionPath = next.getFrozenNode().getPath();
          Resource versionResource = resolver.resolve(versionPath);
          versions.add(new EvolutionImpl(next, versionResource, config));
          log.debug("Version={} added to EvolutionItem", next.getName());
      }
     } catch (UnsupportedRepositoryOperationException e1) {
            log.warn("Could not find version for resource={}", resource.getPath());
      } catch (Exception e) {
          log.error("Could not find versions", e);
      }
      evolutionItems = new ArrayList<>(versions);
  		// evolutionItems.add(new CurrentEvolutionImpl(resource, this.config));
			evolutionItems.add(new CurrentEvolutionImpl(this.resource, this.config));
}

Besides, there are other fields like this.I sorted them out and put them in the table below.

Location Field
com.adobe.acs.commons.version.impl.EvolutionContextImpl config
com.adobe.acs.commons.version.impl.CurrentEvolutionImpl config
com.adobe.acs.commons.reports.models.ReferencesModel resource
com.adobe.acs.commons.util.impl.ComponentDisabler disabledComponents
com.adobe.acs.commons.twitter.impl.TwitterAdapterFactory httpProxyHost
com.adobe.acs.commons.twitter.impl.TwitterAdapterFactory httpProxyPort
com.adobe.acs.commons.twitter.impl.TwitterAdapterFactory useSsl
com.adobe.acs.commons.reports.models.ReportRunner page
com.adobe.acs.commons.wcm.comparisons.impl.lines.LinesGenerator leftSpacer
com.adobe.acs.commons.wcm.comparisons.impl.lines.LinesGenerator left
com.adobe.acs.commons.wcm.comparisons.impl.lines.LinesGenerator rightSpacer
com.adobe.acs.commons.wcm.comparisons.impl.lines.LinesGenerator right

Expected Behavior

Some fields can be replaced by local variable.

Actual Behavior

Use fields as local variables.

Steps to Reproduce

See attachment for details
acs-aem-commons.md

Links

See attachment for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants