component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Jacques Le Roux Mon, 11 Dec 2017 00:32:52 -0800

Hi Michael,

I'm not sure about all the other commits for "General refactoring and code 
improvements" (subtasks of OFBIZ-9836)


But here I found a lot of unnecessary changes and this makes things a bit 
harder to review.

Why all these changes in JavaDoc? Is that related with EOLs? If so then something is wrong because I set the SVN repo to handle that automatically athttp://markmail.org/message/gzghk44kyz646e4d. Maybe you and/or Dennis are not using a svn >= 1.8 client?

Also for OFBIZ-9872 the Jira lacks a description to possibly understand why theses changes (notably why you rightly removed the "if(Debug.verboseOn())" test because it's done at the bottom level with "if (isOn(level)) {"


Maybe some could find this last change disputable for performance reason (use 
of String concatenation when logging message).
But if that was true with older Java versions it's no longer a problem (apart 
in loops) and it's easier to read. We can (should) trust the compiler ;)
https://stackoverflow.com/questions/21805557/string-format-vs-string-concatenation-performance
http://www.java67.com/2015/05/4-ways-to-concatenate-strings-in-java.html (see 
1st comment)

Thanks for your work!

Jacques


Le 11/12/2017 à 08:54, mbr...@apache.org a écrit :

Author: mbrohl
Date: Mon Dec 11 07:54:13 2017
New Revision: 1817750

URL: http://svn.apache.org/viewvc?rev=1817750&view=rev
Log:
Improved: General refactoring and code improvements, package
org.apache.ofbiz.base.component.
(OFBIZ-9872)

The patches were modified to remove unnecessary Debug.is[Loglevel]On()
conditions, see OFBIZ-10049.

Thanks Dennis Balkir for reporting and providing the patches.

Modified:
     
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
     
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
     
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java

Modified: 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java?rev=1817750&r1=1817749&r2=1817750&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
 Mon Dec 11 07:54:13 2017
@@ -47,7 +47,7 @@ import org.w3c.dom.Element;

/**

   * An object that models the <code>&lt;ofbiz-component&gt;</code> element.
- *
+ *
   * @see <code>ofbiz-component.xsd</code>
   *
   */
@@ -458,8 +458,7 @@ public final class ComponentConfig {
          } catch (ContainerException ce) {
              throw new ComponentException("Error reading container configurations 
for component: " + this.globalName, ce);
          }
-        if (Debug.verboseOn())
-            Debug.logVerbose("Read component config : [" + rootLocation + "]", 
module);
+        Debug.logVerbose("Read component config : [" + rootLocation + "]", 
module);
      }

public boolean enabled() {

@@ -596,7 +595,7 @@ public final class ComponentConfig {

/**

       * An object that models the <code>&lt;classpath&gt;</code> element.
-     *
+     *
       * @see <code>ofbiz-component.xsd</code>
       *
       */
@@ -619,7 +618,7 @@ public final class ComponentConfig {
          private final Map<String, ComponentConfig> componentConfigs = new 
LinkedHashMap<>();
          // Root location mapped to global name.
          private final Map<String, String> componentLocations = new 
HashMap<>();
-
+
          private synchronized ComponentConfig fromGlobalName(String 
globalName) {
              return componentConfigs.get(globalName);
          }
@@ -646,7 +645,7 @@ public final class ComponentConfig {

/**

       * An object that models the <code>&lt;entity-resource&gt;</code> element.
-     *
+     *
       * @see <code>ofbiz-component.xsd</code>
       *
       */
@@ -663,7 +662,7 @@ public final class ComponentConfig {

/**

       * An object that models the <code>&lt;keystore&gt;</code> element.
-     *
+     *
       * @see <code>ofbiz-component.xsd</code>
       *
       */
@@ -740,7 +739,7 @@ public final class ComponentConfig {

/**

       * An object that models the <code>&lt;resource-loader&gt;</code> element.
-     *
+     *
       * @see <code>ofbiz-component.xsd</code>
       *
       */
@@ -760,7 +759,7 @@ public final class ComponentConfig {

/**

       * An object that models the <code>&lt;service-resource&gt;</code> 
element.
-     *
+     *
       * @see <code>ofbiz-component.xsd</code>
       *
       */
@@ -775,7 +774,7 @@ public final class ComponentConfig {

/**

       * An object that models the <code>&lt;test-suite&gt;</code> element.
-     *
+     *
       * @see <code>ofbiz-component.xsd</code>
       *
       */
@@ -787,7 +786,7 @@ public final class ComponentConfig {

/**

       * An object that models the <code>&lt;webapp&gt;</code> element.
-     *
+     *
       * @see <code>ofbiz-component.xsd</code>
       *
       */

Modified: 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java?rev=1817750&r1=1817749&r2=1817750&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
 Mon Dec 11 07:54:13 2017
@@ -55,7 +55,7 @@ public final class ComponentLoaderConfig
      public static List<ComponentDef> getComponentsFromConfig(URL configUrl) 
throws ComponentException {
          Document document = parseDocumentFromUrl(configUrl);
          List<? extends Element> toLoad = 
UtilXml.childElementList(document.getDocumentElement());
-        List<ComponentDef> componentsFromConfig = new 
ArrayList<ComponentDef>();
+        List<ComponentDef> componentsFromConfig = new ArrayList<>();

for (Element element : toLoad) {

              componentsFromConfig.add(retrieveComponentDefFromElement(element, 
configUrl));

Modified: 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java?rev=1817750&r1=1817749&r2=1817750&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java
 Mon Dec 11 07:54:13 2017
@@ -18,15 +18,19 @@
   
*******************************************************************************/
  package org.apache.ofbiz.base.component;

+import java.io.IOException;

  import java.io.InputStream;
  import java.net.URL;

+import javax.xml.parsers.ParserConfigurationException;

+
  import org.apache.ofbiz.base.config.GenericConfigException;
  import org.apache.ofbiz.base.config.ResourceHandler;
  import org.apache.ofbiz.base.util.Debug;
  import org.apache.ofbiz.base.util.UtilXml;
  import org.w3c.dom.Document;
  import org.w3c.dom.Element;
+import org.xml.sax.SAXException;

/**

   * Contains resource information and provides for loading data
@@ -50,7 +54,7 @@ public class ComponentResourceHandler im
          this.componentName = componentName;
          this.loaderName = loaderName;
          this.location = location;
-        if (Debug.verboseOn()) Debug.logVerbose("Created " + this.toString(), 
module);
+        Debug.logVerbose("Created " + this.toString(), module);
      }

public String getLoaderName() {

@@ -64,11 +68,7 @@ public class ComponentResourceHandler im
      public Document getDocument() throws GenericConfigException {
          try {
              return UtilXml.readXmlDocument(this.getStream(), 
this.getFullLocation(), true);
-        } catch (org.xml.sax.SAXException e) {
-            throw new GenericConfigException("Error reading " + 
this.toString(), e);
-        } catch (javax.xml.parsers.ParserConfigurationException e) {
-            throw new GenericConfigException("Error reading " + 
this.toString(), e);
-        } catch (java.io.IOException e) {
+        } catch (SAXException | ParserConfigurationException | IOException  e) 
{
              throw new GenericConfigException("Error reading " + 
this.toString(), e);
          }
      }

Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Reply via email to