[GitHub] [commons-crypto] garydgregory commented on a change in pull request #129: Minor Improvements:

[GitBox <gi...@apache.org>]

garydgregory commented on a change in pull request #129:
URL: https://github.com/apache/commons-crypto/pull/129#discussion_r553931692



##########
File path: src/main/java/org/apache/commons/crypto/utils/ReflectionUtils.java
##########
@@ -30,7 +32,7 @@
  */
 public final class ReflectionUtils {
 
-    private static final Map<ClassLoader, Map<String, WeakReference<Class<?>>>> CACHE_CLASSES = new WeakHashMap<>();
+    private static final ConcurrentMap<ClassLoader, Map<String, WeakReference<Class<?>>>> CACHE_CLASSES = new ConcurrentHashMap<>();

Review comment:
       Why?

##########
File path: src/main/java/org/apache/commons/crypto/cipher/JceCipher.java
##########
@@ -35,6 +35,9 @@
  * Implements the {@link CryptoCipher} using JCE provider.
  */
 class JceCipher implements CryptoCipher {
+    /**
+     *  The Cipher {@link Cipher} object.

Review comment:
       Double word.

##########
File path: pom.xml
##########
@@ -731,5 +731,18 @@ The following provides more details on the included cryptographic software:
       <artifactId>junit-jupiter</artifactId>
       <scope>test</scope>
     </dependency>
+    <!-- benchmark -->
+    <dependency>
+      <groupId>org.openjdk.jmh</groupId>
+      <artifactId>jmh-core</artifactId>
+      <version>${jmh.version}</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.openjdk.jmh</groupId>
+      <artifactId>jmh-generator-annprocess</artifactId>
+      <version>${jmh.version}</version>
+      <scope>test</scope>
+    </dependency>

Review comment:
       Why add this if there are no benchmarks in this PR?

##########
File path: src/test/java/org/apache/commons/crypto/stream/AbstractCipherStreamTest.java
##########
@@ -60,11 +60,11 @@
     protected final byte[] data = new byte[dataLen];
     protected byte[] encData;
     private final Properties props = new Properties();
-    protected byte[] key = new byte[16];
-    protected byte[] iv = new byte[16];
-    protected int count = 10000;
-    protected static int defaultBufferSize = 8192;
-    protected static int smallBufferSize = 1024;
+    protected final byte[] key = new byte[16];
+    protected final byte[] iv = new byte[16];
+    protected final int count = 10000;
+    protected static final int defaultBufferSize = 8192;
+    protected static final int smallBufferSize = 1024;

Review comment:
       Don't break binary compatibility.




----------------------------------------------------------------
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