You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Henri Yandell (JIRA)" <ji...@apache.org> on 2006/12/22 20:43:21 UTC

[jira] Created: (LANG-301) Unnecessary code in StrTokenizer

Unnecessary code in StrTokenizer
--------------------------------

                 Key: LANG-301
                 URL: http://issues.apache.org/jira/browse/LANG-301
             Project: Commons Lang
          Issue Type: Bug
    Affects Versions: 2.2
            Reporter: Henri Yandell
             Fix For: 2.3


In StrTokenizer, we have the following:

1086     public Object clone() {
1087         try {
1088             return cloneReset();
1089         } catch (CloneNotSupportedException ex) {
1090             return null;
1091         }
1092     }
...
1101     Object cloneReset() throws CloneNotSupportedException {
1102         StrTokenizer cloned = (StrTokenizer) super.clone();
1103         if (cloned.chars != null) {
1104             cloned.chars = (char[]) cloned.chars.clone();
1105         }
1106         cloned.reset();
1107         return cloned;
1108     }

FindBugs just reported it because the clone() method doesn't call super.clone(). While that's not a worry (because the method it calls does), I don't understand why we're not just doing:

    public Object clone() {
         StrTokenizer cloned = (StrTokenizer) super.clone();
         if (cloned.chars != null) {
             cloned.chars = (char[]) cloned.chars.clone();
         }
         cloned.reset();
         return cloned;
    }

and why we return null and not a runtime CloneNotSupportedException.

Also, is there any value in StrTokenizer being Cloneable? Or is it just done for the sake of the getXxxInstance methods?

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[jira] Resolved: (LANG-301) Unnecessary code in StrTokenizer

Posted by "Stephen Colebourne (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/LANG-301?page=all ]

Stephen Colebourne resolved LANG-301.
-------------------------------------

    Fix Version/s:     (was: 3.0)
       Resolution: Invalid

The cloneReset() method exists to enable testing.

The return null should never happen, as super.clone() should succeed. Its really just a way around the JDK flaw of the exception not being a runtime exception.

> Unnecessary code in StrTokenizer
> --------------------------------
>
>                 Key: LANG-301
>                 URL: http://issues.apache.org/jira/browse/LANG-301
>             Project: Commons Lang
>          Issue Type: Bug
>    Affects Versions: 2.2
>            Reporter: Henri Yandell
>
> In StrTokenizer, we have the following:
> 1086     public Object clone() {
> 1087         try {
> 1088             return cloneReset();
> 1089         } catch (CloneNotSupportedException ex) {
> 1090             return null;
> 1091         }
> 1092     }
> ...
> 1101     Object cloneReset() throws CloneNotSupportedException {
> 1102         StrTokenizer cloned = (StrTokenizer) super.clone();
> 1103         if (cloned.chars != null) {
> 1104             cloned.chars = (char[]) cloned.chars.clone();
> 1105         }
> 1106         cloned.reset();
> 1107         return cloned;
> 1108     }
> FindBugs just reported it because the clone() method doesn't call super.clone(). While that's not a worry (because the method it calls does), I don't understand why we're not just doing:
>     public Object clone() {
>          StrTokenizer cloned = (StrTokenizer) super.clone();
>          if (cloned.chars != null) {
>              cloned.chars = (char[]) cloned.chars.clone();
>          }
>          cloned.reset();
>          return cloned;
>     }
> and why we return null and not a runtime CloneNotSupportedException.
> Also, is there any value in StrTokenizer being Cloneable? Or is it just done for the sake of the getXxxInstance methods?

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[jira] Updated: (LANG-301) Unnecessary code in StrTokenizer

Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/LANG-301?page=all ]

Henri Yandell updated LANG-301:
-------------------------------

    Fix Version/s: 3.0
                       (was: 2.3)

Moving this back to 3.0. 

There are three things to do here, if reasons for the current code don't show up:

1) Refactor the cloneReset method away. It's possible that it's setup this way to be more useful for subclasses. 
2) Switch from returning null to throwing CloneNotSupported.
3) Stop supporting clone() and Cloneable.

The latter 2 definitely need to wait for 3.0, the former is not very critical.

> Unnecessary code in StrTokenizer
> --------------------------------
>
>                 Key: LANG-301
>                 URL: http://issues.apache.org/jira/browse/LANG-301
>             Project: Commons Lang
>          Issue Type: Bug
>    Affects Versions: 2.2
>            Reporter: Henri Yandell
>             Fix For: 3.0
>
>
> In StrTokenizer, we have the following:
> 1086     public Object clone() {
> 1087         try {
> 1088             return cloneReset();
> 1089         } catch (CloneNotSupportedException ex) {
> 1090             return null;
> 1091         }
> 1092     }
> ...
> 1101     Object cloneReset() throws CloneNotSupportedException {
> 1102         StrTokenizer cloned = (StrTokenizer) super.clone();
> 1103         if (cloned.chars != null) {
> 1104             cloned.chars = (char[]) cloned.chars.clone();
> 1105         }
> 1106         cloned.reset();
> 1107         return cloned;
> 1108     }
> FindBugs just reported it because the clone() method doesn't call super.clone(). While that's not a worry (because the method it calls does), I don't understand why we're not just doing:
>     public Object clone() {
>          StrTokenizer cloned = (StrTokenizer) super.clone();
>          if (cloned.chars != null) {
>              cloned.chars = (char[]) cloned.chars.clone();
>          }
>          cloned.reset();
>          return cloned;
>     }
> and why we return null and not a runtime CloneNotSupportedException.
> Also, is there any value in StrTokenizer being Cloneable? Or is it just done for the sake of the getXxxInstance methods?

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org