[jira] Created: (LANG-427) StringUtils code readability (method acceleration)

StringUtils code readability (method acceleration)
--------------------------------------------------

                 Key: LANG-427
                 URL: https://issues.apache.org/jira/browse/LANG-427
             Project: Commons Lang
          Issue Type: Improvement
         Environment: All systems, all plattforms
            Reporter: Torsten
            Priority: Trivial


First of all i hope you will let me stay alive :) because this is not a critical error or even a bug. My proposal will not help to make the method run twice as fast, but I think this would make the code easier to read, test and to maintain. I tested both methods with Strings of length 1 to 10000 and with growing size of the Strings, the second method got actual faster and it *never* was slower.

This method is from the framework:
{code:title=StringUtils.java|borderStyle=solid}
    public static boolean isBlank(String str) {
        int strLen;
        if (str == null || (strLen = str.length()) == 0) {
            return true;
        }
        for (int i = 0; i < strLen; i++) {
            if ((Character.isWhitespace(str.charAt(i)) == false)) {
                return false;
            }
        }
        return true;
    }
{code}

I think this method would have the same result:
{code:borderStyle=solid}
    public static final boolean isBlank(String str) {
        return str == null || str.trim().length() == 0;
    }
{code}

Greetings from Germany
Torsten

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/LANG-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12591057#action_12591057 ]

Robert Scholte commented on LANG-427:
-------------------------------------

I can't see which unittests you've written, so it's hard to make conclusions yet.
Doesn't it have to do with an optimistic or pessimistic approach? If you're optimistic and expects the string to be blank your approach is probably faster.
But if you expect the string not to be blank the original version is faster, I guess. (or should you use isNotBlank() in this case?)
And about the readablility: both implementations are clear enough. But I guess the speed usualy prio one

> StringUtils code readability (method acceleration)
> --------------------------------------------------
>
>                 Key: LANG-427
>                 URL: https://issues.apache.org/jira/browse/LANG-427
>             Project: Commons Lang
>          Issue Type: Improvement
>         Environment: All systems, all plattforms
>            Reporter: Torsten
>            Priority: Trivial
>   Original Estimate: 0.08h
>  Remaining Estimate: 0.08h
>
> First of all i hope you will let me stay alive :) because this is not a critical error or even a bug. My proposal will not help to make the method run twice as fast, but I think this would make the code easier to read, test and to maintain. I tested both methods with Strings of length 1 to 10000 and with growing size of the Strings, the second method got actual faster and it *never* was slower.
> This method is from the framework:
> {code:title=StringUtils.java|borderStyle=solid}
>     public static boolean isBlank(String str) {
>         int strLen;
>         if (str == null || (strLen = str.length()) == 0) {
>             return true;
>         }
>         for (int i = 0; i < strLen; i++) {
>             if ((Character.isWhitespace(str.charAt(i)) == false)) {
>                 return false;
>             }
>         }
>         return true;
>     }
> {code}
> I think this method would have the same result:
> {code:borderStyle=solid}
>     public static final boolean isBlank(String str) {
> return str == null || str.trim().length() == 0;
>     }
> {code}
> Greetings from Germany
> Torsten

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/LANG-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12591057#action_12591057 ]

rfscholte-getthere edited comment on LANG-427 at 4/21/08 2:03 PM:
--------------------------------------------------------------

I can't see which unittests you've written, so it's hard to make conclusions yet.
Doesn't it have to do with an optimistic or pessimistic approach? If you're optimistic and expects the string to be blank your approach is probably faster.
But if you expect the string not to be blank the original version is faster, I guess. (or should you use isNotBlank() in this case?)
And about the readablility: both implementations are clear enough. But I guess the speed is usualy prio one


      was (Author: rfscholte-getthere):
    I can't see which unittests you've written, so it's hard to make conclusions yet.
Doesn't it have to do with an optimistic or pessimistic approach? If you're optimistic and expects the string to be blank your approach is probably faster.
But if you expect the string not to be blank the original version is faster, I guess. (or should you use isNotBlank() in this case?)
And about the readablility: both implementations are clear enough. But I guess the speed usualy prio one

 

> StringUtils code readability (method acceleration)
> --------------------------------------------------
>
>                 Key: LANG-427
>                 URL: https://issues.apache.org/jira/browse/LANG-427
>             Project: Commons Lang
>          Issue Type: Improvement
>         Environment: All systems, all plattforms
>            Reporter: Torsten
>            Priority: Trivial
>   Original Estimate: 0.08h
>  Remaining Estimate: 0.08h
>
> First of all i hope you will let me stay alive :) because this is not a critical error or even a bug. My proposal will not help to make the method run twice as fast, but I think this would make the code easier to read, test and to maintain. I tested both methods with Strings of length 1 to 10000 and with growing size of the Strings, the second method got actual faster and it *never* was slower.
> This method is from the framework:
> {code:title=StringUtils.java|borderStyle=solid}
>     public static boolean isBlank(String str) {
>         int strLen;
>         if (str == null || (strLen = str.length()) == 0) {
>             return true;
>         }
>         for (int i = 0; i < strLen; i++) {
>             if ((Character.isWhitespace(str.charAt(i)) == false)) {
>                 return false;
>             }
>         }
>         return true;
>     }
> {code}
> I think this method would have the same result:
> {code:borderStyle=solid}
>     public static final boolean isBlank(String str) {
> return str == null || str.trim().length() == 0;
>     }
> {code}
> Greetings from Germany
> Torsten

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/LANG-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12595892#action_12595892 ]

Henri Yandell commented on LANG-427:
------------------------------------

Looks to me like it'll be different if your text contains characters matching:

"It is a Unicode space character (SPACE_SEPARATOR, LINE_SEPARATOR, or PARAGRAPH_SEPARATOR) but is not also a non-breaking space ('\u00A0', '\u2007', '\u202F')."

It looks like they return true for isWhitespace, but are not removed by trim.

I think we should WONTFIX this issue as while it's presumably not hugely common to see those three characters in an isBlank, it would represent a change of functionality for no strong reason.

> StringUtils code readability (method acceleration)
> --------------------------------------------------
>
>                 Key: LANG-427
>                 URL: https://issues.apache.org/jira/browse/LANG-427
>             Project: Commons Lang
>          Issue Type: Improvement
>         Environment: All systems, all plattforms
>            Reporter: Torsten
>            Priority: Trivial
>   Original Estimate: 0.08h
>  Remaining Estimate: 0.08h
>
> First of all i hope you will let me stay alive :) because this is not a critical error or even a bug. My proposal will not help to make the method run twice as fast, but I think this would make the code easier to read, test and to maintain. I tested both methods with Strings of length 1 to 10000 and with growing size of the Strings, the second method got actual faster and it *never* was slower.
> This method is from the framework:
> {code:title=StringUtils.java|borderStyle=solid}
>     public static boolean isBlank(String str) {
>         int strLen;
>         if (str == null || (strLen = str.length()) == 0) {
>             return true;
>         }
>         for (int i = 0; i < strLen; i++) {
>             if ((Character.isWhitespace(str.charAt(i)) == false)) {
>                 return false;
>             }
>         }
>         return true;
>     }
> {code}
> I think this method would have the same result:
> {code:borderStyle=solid}
>     public static final boolean isBlank(String str) {
> return str == null || str.trim().length() == 0;
>     }
> {code}
> Greetings from Germany
> Torsten

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/LANG-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12595896#action_12595896 ]

Torsten commented on LANG-427:
------------------------------

Hello and thank you for your reply. The performance tests showed that it is faster on my computer to use "my" code in an optimistic and in an  pessimistic approach. But yes, it is right. The methods are not equal. I tested both methods and there are some differences. I'm sorry! Maybe someone can use my code anyway :)

Here is a list of the characters causing different results:
charVal="\u0000" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0001" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0002" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0003" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0004" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0005" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0006" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0007" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0008" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u000E" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u000F" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0010" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0011" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0012" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0013" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0014" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0015" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0016" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0017" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0018" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u0019" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u001A" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u001B" | StringUtils.isBlank="false" | Other.isBlank="true "
charVal="\u1680" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u180E" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2000" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2001" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2002" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2003" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2004" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2005" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2006" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2008" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2009" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u200A" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u200B" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2028" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u2029" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u205F" | StringUtils.isBlank="true " | Other.isBlank="false"
charVal="\u3000" | StringUtils.isBlank="true " | Other.isBlank="false"



Greetings
Torsten

> StringUtils code readability (method acceleration)
> --------------------------------------------------
>
>                 Key: LANG-427
>                 URL: https://issues.apache.org/jira/browse/LANG-427
>             Project: Commons Lang
>          Issue Type: Improvement
>         Environment: All systems, all plattforms
>            Reporter: Torsten
>            Priority: Trivial
>   Original Estimate: 0.08h
>  Remaining Estimate: 0.08h
>
> First of all i hope you will let me stay alive :) because this is not a critical error or even a bug. My proposal will not help to make the method run twice as fast, but I think this would make the code easier to read, test and to maintain. I tested both methods with Strings of length 1 to 10000 and with growing size of the Strings, the second method got actual faster and it *never* was slower.
> This method is from the framework:
> {code:title=StringUtils.java|borderStyle=solid}
>     public static boolean isBlank(String str) {
>         int strLen;
>         if (str == null || (strLen = str.length()) == 0) {
>             return true;
>         }
>         for (int i = 0; i < strLen; i++) {
>             if ((Character.isWhitespace(str.charAt(i)) == false)) {
>                 return false;
>             }
>         }
>         return true;
>     }
> {code}
> I think this method would have the same result:
> {code:borderStyle=solid}
>     public static final boolean isBlank(String str) {
> return str == null || str.trim().length() == 0;
>     }
> {code}
> Greetings from Germany
> Torsten

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/LANG-427?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henri Yandell closed LANG-427.
------------------------------

    Resolution: Won't Fix

Marking as WONTFIX as there would be functionality changes.

> StringUtils code readability (method acceleration)
> --------------------------------------------------
>
>                 Key: LANG-427
>                 URL: https://issues.apache.org/jira/browse/LANG-427
>             Project: Commons Lang
>          Issue Type: Improvement
>         Environment: All systems, all plattforms
>            Reporter: Torsten
>            Priority: Trivial
>   Original Estimate: 0.08h
>  Remaining Estimate: 0.08h
>
> First of all i hope you will let me stay alive :) because this is not a critical error or even a bug. My proposal will not help to make the method run twice as fast, but I think this would make the code easier to read, test and to maintain. I tested both methods with Strings of length 1 to 10000 and with growing size of the Strings, the second method got actual faster and it *never* was slower.
> This method is from the framework:
> {code:title=StringUtils.java|borderStyle=solid}
>     public static boolean isBlank(String str) {
>         int strLen;
>         if (str == null || (strLen = str.length()) == 0) {
>             return true;
>         }
>         for (int i = 0; i < strLen; i++) {
>             if ((Character.isWhitespace(str.charAt(i)) == false)) {
>                 return false;
>             }
>         }
>         return true;
>     }
> {code}
> I think this method would have the same result:
> {code:borderStyle=solid}
>     public static final boolean isBlank(String str) {
> return str == null || str.trim().length() == 0;
>     }
> {code}
> Greetings from Germany
> Torsten

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/LANG-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12596351#action_12596351 ]

Julien Aymé commented on LANG-427:
----------------------------------

I have another comment regarding the fact that the new method is never slower:

IMO, the new method can be slower for the non-empty strings, since the trim() method will try to remove spaces at the  beginning and at the end of the string, whereas isWhiteSpace will return false for the first character.

The new method will certainly be slower for non-empty strings with lots of white spaces at the end:
in addition of the argument mentioned above, the trim() method has to create a new String (the trimmed one), whereas the current code does not create any new object.

> StringUtils code readability (method acceleration)
> --------------------------------------------------
>
>                 Key: LANG-427
>                 URL: https://issues.apache.org/jira/browse/LANG-427
>             Project: Commons Lang
>          Issue Type: Improvement
>         Environment: All systems, all plattforms
>            Reporter: Torsten
>            Priority: Trivial
>   Original Estimate: 0.08h
>  Remaining Estimate: 0.08h
>
> First of all i hope you will let me stay alive :) because this is not a critical error or even a bug. My proposal will not help to make the method run twice as fast, but I think this would make the code easier to read, test and to maintain. I tested both methods with Strings of length 1 to 10000 and with growing size of the Strings, the second method got actual faster and it *never* was slower.
> This method is from the framework:
> {code:title=StringUtils.java|borderStyle=solid}
>     public static boolean isBlank(String str) {
>         int strLen;
>         if (str == null || (strLen = str.length()) == 0) {
>             return true;
>         }
>         for (int i = 0; i < strLen; i++) {
>             if ((Character.isWhitespace(str.charAt(i)) == false)) {
>                 return false;
>             }
>         }
>         return true;
>     }
> {code}
> I think this method would have the same result:
> {code:borderStyle=solid}
>     public static final boolean isBlank(String str) {
> return str == null || str.trim().length() == 0;
>     }
> {code}
> Greetings from Germany
> Torsten

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.