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/03/04 16:52:12 UTC

[GitHub] [commons-codec] arturobernalg opened a new pull request #78: Validate byte and string

arturobernalg opened a new pull request #78:
URL: https://github.com/apache/commons-codec/pull/78


   Create a method to validate if a CharSequence is empty ("") or null
   Make public BinaryCodec.isEmpty in order to reuse and avoid duplicate validations 


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



[GitHub] [commons-codec] garydgregory commented on a change in pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#discussion_r806300692



##########
File path: src/main/java/org/apache/commons/codec/binary/StringUtils.java
##########
@@ -269,6 +269,25 @@ public static ByteBuffer getByteBufferUtf8(final String string) {
         return getBytes(string, StandardCharsets.UTF_8);
     }
 
+    /**
+     * <p>Checks if a CharSequence is empty ("") or null.</p>
+     *
+     * <pre>
+     * StringUtils.isEmpty(null)      = true
+     * StringUtils.isEmpty("")        = true
+     * StringUtils.isEmpty(" ")       = false
+     * StringUtils.isEmpty("bob")     = false
+     * StringUtils.isEmpty("  bob  ") = false
+     * </pre>
+     *
+     * @param cs  the CharSequence to check, may be null
+     * @return {@code true} if the CharSequence is empty or null
+     * @since 1.16
+     */
+    public static boolean isEmpty(final CharSequence cs) {

Review comment:
       Make this package-private and move it somewhere in the language package since it is the only place where it seems to be called, which will help as noted in my other comment.

##########
File path: src/main/java/org/apache/commons/codec/binary/BinaryCodec.java
##########
@@ -136,8 +136,10 @@
      * @param array
      *            the source array
      * @return {@code true} if the given array is {@code null} or empty (size 0.)
+     *
+     * @since 1.16 change visibility to public
      */
-    private static boolean isEmpty(final byte[] array) {
+    public static boolean isEmpty(final byte[] array) {

Review comment:
       Hello @arturobernalg 
   Please make this package private instead of public, this helps minimize the API footprint we will need to maintain for BC, especially since this is just for internal use.




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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] garydgregory commented on a change in pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#discussion_r810507274



##########
File path: src/main/java/org/apache/commons/codec/binary/StringUtils.java
##########
@@ -269,6 +269,25 @@ public static ByteBuffer getByteBufferUtf8(final String string) {
         return getBytes(string, StandardCharsets.UTF_8);
     }
 
+    /**
+     * <p>Checks if a CharSequence is empty ("") or null.</p>
+     *
+     * <pre>
+     * StringUtils.isEmpty(null)      = true
+     * StringUtils.isEmpty("")        = true
+     * StringUtils.isEmpty(" ")       = false
+     * StringUtils.isEmpty("bob")     = false
+     * StringUtils.isEmpty("  bob  ") = false
+     * </pre>
+     *
+     * @param cs  the CharSequence to check, may be null
+     * @return {@code true} if the CharSequence is empty or null
+     * @since 1.16
+     */
+    public static boolean isEmpty(final CharSequence cs) {

Review comment:
       Hi @arturobernalg ping?




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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] arturobernalg commented on a change in pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#discussion_r810508855



##########
File path: src/main/java/org/apache/commons/codec/binary/StringUtils.java
##########
@@ -269,6 +269,25 @@ public static ByteBuffer getByteBufferUtf8(final String string) {
         return getBytes(string, StandardCharsets.UTF_8);
     }
 
+    /**
+     * <p>Checks if a CharSequence is empty ("") or null.</p>
+     *
+     * <pre>
+     * StringUtils.isEmpty(null)      = true
+     * StringUtils.isEmpty("")        = true
+     * StringUtils.isEmpty(" ")       = false
+     * StringUtils.isEmpty("bob")     = false
+     * StringUtils.isEmpty("  bob  ") = false
+     * </pre>
+     *
+     * @param cs  the CharSequence to check, may be null
+     * @return {@code true} if the CharSequence is empty or null
+     * @since 1.16
+     */
+    public static boolean isEmpty(final CharSequence cs) {

Review comment:
       changed 




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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] garydgregory merged pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #78:
URL: https://github.com/apache/commons-codec/pull/78


   


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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] arturobernalg commented on a change in pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#discussion_r812241151



##########
File path: src/test/java/org/apache/commons/codec/binary/StringUtilsTest.java
##########
@@ -121,6 +121,7 @@ public void testGetBytesUncheckedNullInput() {
         Assert.assertNull(StringUtils.getBytesUnchecked(null, "UNKNOWN"));
     }
 
+

Review comment:
       removed.




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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] arturobernalg commented on pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#issuecomment-1048110711


   > Hi @arturobernalg
   > Would you please resolve the one conflict?
   
   done


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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] arturobernalg commented on pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#issuecomment-1046182047


   > Hi @arturobernal
   > Would you mind rebasing this PR on master?
   
   
   
   > Hi @arturobernal
   > Would you mind rebasing this PR on master?
   
   Done.


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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] garydgregory commented on a change in pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#discussion_r806300692



##########
File path: src/main/java/org/apache/commons/codec/binary/StringUtils.java
##########
@@ -269,6 +269,25 @@ public static ByteBuffer getByteBufferUtf8(final String string) {
         return getBytes(string, StandardCharsets.UTF_8);
     }
 
+    /**
+     * <p>Checks if a CharSequence is empty ("") or null.</p>
+     *
+     * <pre>
+     * StringUtils.isEmpty(null)      = true
+     * StringUtils.isEmpty("")        = true
+     * StringUtils.isEmpty(" ")       = false
+     * StringUtils.isEmpty("bob")     = false
+     * StringUtils.isEmpty("  bob  ") = false
+     * </pre>
+     *
+     * @param cs  the CharSequence to check, may be null
+     * @return {@code true} if the CharSequence is empty or null
+     * @since 1.16
+     */
+    public static boolean isEmpty(final CharSequence cs) {

Review comment:
       Make this package-private and move it somewhere in the language package since it is the only place where it seems to be called, which will help as noted in my other comment.

##########
File path: src/main/java/org/apache/commons/codec/binary/BinaryCodec.java
##########
@@ -136,8 +136,10 @@
      * @param array
      *            the source array
      * @return {@code true} if the given array is {@code null} or empty (size 0.)
+     *
+     * @since 1.16 change visibility to public
      */
-    private static boolean isEmpty(final byte[] array) {
+    public static boolean isEmpty(final byte[] array) {

Review comment:
       Hello @arturobernalg 
   Please make this package private instead of public, this helps minimize the API footprint we will need to maintain for BC, especially since this is just for internal use.




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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] garydgregory commented on pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#issuecomment-1046133919


   Hi @arturobernal
   Would you mind rebasing this PR on master?


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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] garydgregory commented on a change in pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#discussion_r810691455



##########
File path: src/test/java/org/apache/commons/codec/binary/StringUtilsTest.java
##########
@@ -121,6 +121,7 @@ public void testGetBytesUncheckedNullInput() {
         Assert.assertNull(StringUtils.getBytesUnchecked(null, "UNKNOWN"));
     }
 
+

Review comment:
       I do not think we need this extra blank line.




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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] garydgregory commented on pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#issuecomment-1048108862


   Hi @arturobernalg 
   Would you please resolve the one conflict?


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

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-codec] coveralls commented on pull request #78: Validate byte and string

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #78:
URL: https://github.com/apache/commons-codec/pull/78#issuecomment-790787679


   
   [![Coverage Status](https://coveralls.io/builds/37660050/badge)](https://coveralls.io/builds/37660050)
   
   Coverage increased (+0.001%) to 94.51% when pulling **35a4093d144476f96df2d0226805a01b272c80e1 on arturobernalg:feature/utils** into **85cb447f13280ee29c9311a0c78a26f6d10d44dd on apache:master**.
   


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