commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r826960 - /commons/proper/lang/trunk/src/java/org/apache/commons/lang/ObjectUtils.java
Date Mon, 26 Oct 2009 10:29:29 GMT
On 25/10/2009, Henri Yandell <flamefew@gmail.com> wrote:
> On Sun, Oct 25, 2009 at 4:02 AM, sebb <sebbaz@gmail.com> wrote:
>  > On 25/10/2009, Henri Yandell <flamefew@gmail.com> wrote:
>  >> On Sat, Oct 24, 2009 at 4:05 AM, Stephen Colebourne
>  >>  <scolebourne@btopenworld.com> wrote:
>  >>  > bayard@apache.org wrote:
>  >>  >>
>  >>  >> URL: http://svn.apache.org/viewvc?rev=826960&view=rev
>  >>  >> Log:
>  >>  >> Moved identityToString(StringBuffer, Object) to
>  >>  >> identityToString(Appendable, Object) per LANG-542
>  >>  >> @@ -178,13 +178,17 @@
>  >>  >>      * @param object  the object to create a toString for
>  >>  >>      * @since 2.4
>  >>  >>      */
>  >>  >> -    public static void identityToString(StringBuffer buffer, Object
>  >>  >> object) {
>  >>  >> +    public static void identityToString(Appendable buffer, Object
object)
>  >>  >> {
>  >>  >>         if (object == null) {
>  >>  >>             throw new NullPointerException("Cannot get the toString
of a
>  >>  >> null identity");
>  >>  >>         }
>  >>  >> -        buffer.append(object.getClass().getName())
>  >>  >> -              .append('@')
>  >>  >> -
>  >>  >>  .append(Integer.toHexString(System.identityHashCode(object)));
>  >>  >> +        try {
>  >>  >> +            buffer.append(object.getClass().getName())
>  >>  >> +                  .append('@')
>  >>  >> +
>  >>  >>  .append(Integer.toHexString(System.identityHashCode(object)));
>  >>  >> +        } catch(java.io.IOException ioe) {
>  >>  >> +            // can't happen - Appendable API forces it upon us
>  >>  >> +        }
>  >>  >>     }
>  >>  >
>  >>  > This change is invalid.
>  >>  >
>  >>  > IO classes such as Writer implement Appendable, and appending to those
can
>  >>  > throw an IOException.
>  >>
>  >>
>  >> Yup.  I remember telling myself it was fine because we passed a
>  >>  StringBuilder in so the IOException wouldn't happen.... 'cept that's
>  >>  only if someone calls identityToString(Object) :)
>  >>
>  >>  I've rolled back the change for now. I don't like IOException being
>  >>  added to the API. I'm tempted by the somewhat lame solution of a
>  >>  private Appendable method, and explicitly supporting StringBuffer,
>  >>  StringBuilder and StrBuilder.
>  >>
>  >
>  > Unfortunately StringBuilder & StringBuffer don't have a common
>  > accessible parent.
>  > [Seems like a design fault to me]
>  >
>  > Rather than swallowing the Exception, why not convert the IOException
>  > to an unchecked Exception? (and document the restriction)
>  >
>  > The only reason for using Appendable is to support
>  > StringBuilder/Buffer, so if it is used with any other Appendable
>  > classes, let the user beware.
>
>
> Why not just do (StringBuilder, Object); (StringBuffer, Object) and
>  (StrBuilder, Object) if that's the intent?
>

Either will do, but neither is ideal.

However using Appendable + rethrown Exception allows code to be shared
and so generates less code. It's also easier to maintain if there is a
single copy of each method.
Not so important for a short method, but some potential candidates are long.

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

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


Mime
View raw message