You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/01/02 14:53:27 UTC
[GitHub] [commons-configuration] garydgregory commented on a change in pull request #91: Minor Improvements
garydgregory commented on a change in pull request #91:
URL: https://github.com/apache/commons-configuration/pull/91#discussion_r550888036
##########
File path: src/main/java/org/apache/commons/configuration2/web/AppletConfiguration.java
##########
@@ -31,7 +31,7 @@
public class AppletConfiguration extends BaseWebConfiguration
{
/** Stores the wrapped applet.*/
- protected Applet applet;
+ protected final Applet applet;
Review comment:
**Please do NOT break binary compatibility.**
##########
File path: src/main/java/org/apache/commons/configuration2/web/ServletRequestConfiguration.java
##########
@@ -34,7 +34,7 @@
public class ServletRequestConfiguration extends BaseWebConfiguration
{
/** Stores the wrapped request.*/
- protected ServletRequest request;
+ protected final ServletRequest request;
Review comment:
**Please do NOT break binary compatibility.**
##########
File path: src/main/java/org/apache/commons/configuration2/spring/ConfigurationPropertiesFactoryBean.java
##########
@@ -56,7 +56,7 @@
/** Spring resources for loading configurations **/
private Resource[] locations;
- /** @see org.apache.commons.configuration2.AbstractConfiguration#throwExceptionOnMissing **/
+ /** @link org.apache.commons.configuration2.AbstractConfiguration#throwExceptionOnMissing **/
Review comment:
-1 breaks Javadoc, leave as is please.
##########
File path: src/main/java/org/apache/commons/configuration2/web/ServletContextConfiguration.java
##########
@@ -34,7 +34,7 @@
public class ServletContextConfiguration extends BaseWebConfiguration
{
/** Stores the wrapped servlet context.*/
- protected ServletContext context;
+ protected final ServletContext context;
Review comment:
**Please do NOT break binary compatibility.**
##########
File path: src/main/java/org/apache/commons/configuration2/web/ServletConfiguration.java
##########
@@ -34,7 +34,7 @@
public class ServletConfiguration extends BaseWebConfiguration
{
/** Stores a reference to the wrapped {@code ServletConfig}.*/
- protected ServletConfig config;
+ protected final ServletConfig config;
Review comment:
**Please do NOT break binary compatibility.**
##########
File path: src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java
##########
@@ -246,13 +246,13 @@ public PropertiesWriterTestImpl(final ListDelimiterHandler handler) throws IOExc
private static final String CR = System.getProperty("line.separator");
/** The File that we test with */
- private static String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
+ private static final String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
Review comment:
static finals are in UPPER CASE by convention.
##########
File path: src/main/java/org/apache/commons/configuration2/tree/ModelTransaction.java
##########
@@ -361,12 +361,7 @@ Operations fetchOperations(final ImmutableNode target, final int level)
Integer.valueOf(level == LEVEL_UNKNOWN ? level(target)
: level);
Map<ImmutableNode, Operations> levelOperations =
- operations.get(nodeLevel);
- if (levelOperations == null)
- {
- levelOperations = new HashMap<>();
- operations.put(nodeLevel, levelOperations);
- }
+ operations.computeIfAbsent(nodeLevel, k -> new HashMap<>());
Review comment:
Fix formatting.
##########
File path: src/main/java/org/apache/commons/configuration2/web/ServletFilterConfiguration.java
##########
@@ -33,7 +33,7 @@
public class ServletFilterConfiguration extends BaseWebConfiguration
{
/** Stores the wrapped filter config.*/
- protected FilterConfig config;
+ protected final FilterConfig config;
Review comment:
**Please do NOT break binary compatibility.**
##########
File path: src/main/java/org/apache/commons/configuration2/io/FileUtils.java
##########
@@ -33,7 +34,7 @@
/**
* The UTF-8 character set, used to decode octets in URLs.
*/
- private static final Charset UTF8 = Charset.forName("UTF-8");
+ private static final Charset UTF8 = StandardCharsets.UTF_8;
Review comment:
You can now inline this private var.
##########
File path: src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java
##########
@@ -246,13 +246,13 @@ public PropertiesWriterTestImpl(final ListDelimiterHandler handler) throws IOExc
private static final String CR = System.getProperty("line.separator");
/** The File that we test with */
- private static String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
+ private static final String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
- private static String testBasePath = ConfigurationAssert.TEST_DIR.getAbsolutePath();
+ private static final String testBasePath = ConfigurationAssert.TEST_DIR.getAbsolutePath();
Review comment:
static finals are in UPPER CASE by convention.
##########
File path: src/test/java/org/apache/commons/configuration2/tree/TestDefaultExpressionEngine.java
##########
@@ -35,15 +35,15 @@
public class TestDefaultExpressionEngine
{
/** Stores the names of the test nodes representing tables. */
- private static String[] tables =
+ private static final String[] tables =
{ "users", "documents"};
/** Stores the types of the test table nodes. */
- private static String[] tabTypes =
+ private static final String[] tabTypes =
Review comment:
static finals are in UPPER CASE by convention.
##########
File path: src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java
##########
@@ -246,13 +246,13 @@ public PropertiesWriterTestImpl(final ListDelimiterHandler handler) throws IOExc
private static final String CR = System.getProperty("line.separator");
/** The File that we test with */
- private static String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
+ private static final String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
- private static String testBasePath = ConfigurationAssert.TEST_DIR.getAbsolutePath();
+ private static final String testBasePath = ConfigurationAssert.TEST_DIR.getAbsolutePath();
- private static String testBasePath2 = ConfigurationAssert.TEST_DIR.getParentFile().getAbsolutePath();
+ private static final String testBasePath2 = ConfigurationAssert.TEST_DIR.getParentFile().getAbsolutePath();
- private static File testSavePropertiesFile = ConfigurationAssert.getOutFile("testsave.properties");
+ private static final File testSavePropertiesFile = ConfigurationAssert.getOutFile("testsave.properties");
Review comment:
static finals are in UPPER CASE by convention.
##########
File path: src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java
##########
@@ -246,13 +246,13 @@ public PropertiesWriterTestImpl(final ListDelimiterHandler handler) throws IOExc
private static final String CR = System.getProperty("line.separator");
/** The File that we test with */
- private static String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
+ private static final String testProperties = ConfigurationAssert.getTestFile("test.properties").getAbsolutePath();
- private static String testBasePath = ConfigurationAssert.TEST_DIR.getAbsolutePath();
+ private static final String testBasePath = ConfigurationAssert.TEST_DIR.getAbsolutePath();
- private static String testBasePath2 = ConfigurationAssert.TEST_DIR.getParentFile().getAbsolutePath();
+ private static final String testBasePath2 = ConfigurationAssert.TEST_DIR.getParentFile().getAbsolutePath();
Review comment:
static finals are in UPPER CASE by convention.
##########
File path: src/test/java/org/apache/commons/configuration2/tree/TestDefaultExpressionEngine.java
##########
@@ -35,15 +35,15 @@
public class TestDefaultExpressionEngine
{
/** Stores the names of the test nodes representing tables. */
- private static String[] tables =
+ private static final String[] tables =
Review comment:
static finals are in UPPER CASE by convention.
##########
File path: src/test/java/org/apache/commons/configuration2/tree/TestDefaultExpressionEngine.java
##########
@@ -35,15 +35,15 @@
public class TestDefaultExpressionEngine
{
/** Stores the names of the test nodes representing tables. */
- private static String[] tables =
+ private static final String[] tables =
{ "users", "documents"};
/** Stores the types of the test table nodes. */
- private static String[] tabTypes =
+ private static final String[] tabTypes =
{ "system", "application"};
/** Test data fields for the node hierarchy. */
- private static String[][] fields =
+ private static final String[][] fields =
Review comment:
static finals are in UPPER CASE by convention.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org