commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henri Yandell (JIRA)" <j...@apache.org>
Subject [jira] Commented: (LANG-360) Why does appendIdentityToString return null?
Date Sat, 03 Nov 2007 07:50:50 GMT

    [ https://issues.apache.org/jira/browse/LANG-360?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539896
] 

Henri Yandell commented on LANG-360:
------------------------------------

Brain kicks in.... Also, I wouldn't expect:

ObjectUtils.appendIdentityToString(buffer, param1)
                    .appendIdentityToString(buffer, param2)
                    .appendIdentityToString(buffer, param3); 

to even compile. A StringBuffer is returned, not an ObjectUtils. 

Chaining isn't allowed here, the actual code would have been the very ugly and nested:

            ObjectUtils.appendIdentityToString(
                ObjectUtils.appendIdentityToString(
                    ObjectUtils.appendIdentityToString(buffer, param1)
                    , param2)
                , param3);

However that doesn't end up with a NullPointerException. Instead you get a very nasty bug
whereby if param2 is null, then you'll end up with a  StringBuffer just containing the text
of param3. 

Ideally the code should be:

* public static String identityToString(Object)
* public static void appendIdentityToString(StringBuffer, Object)

Sod the StringBuffer creation - bet that was only there so the first method could reuse the
second. The problem with that is that we don't have overloaded return types. So an alternative
would be to deprecate both methods and have:

* public static String identity(Object)
* public static void appendIdentity(StringBuffer, Object)

That would allow us to decide if we think a null Object passed in should return the String
"null" or null in the first case; and result in "null" or "" in the second. 

Alternatively, we could go with an overload of the original name:

* public static String identityToString(Object)
* public static void identityToString(StringBuffer, Object)

but that would make the "null" bit bad if we do it.

Thoughts?

> Why does appendIdentityToString return null?
> --------------------------------------------
>
>                 Key: LANG-360
>                 URL: https://issues.apache.org/jira/browse/LANG-360
>             Project: Commons Lang
>          Issue Type: Bug
>            Reporter: Jörg Gottschling
>             Fix For: 2.4
>
>
> ObjectUtils is designed to handle null imputs gracefully. But ObjectUtils.appendIdentityToString
does not. It returns null unnessecary if you pass null als second parameter (the object to
get the identity from). For example appendIdentityToString(new StringBuffer(), null) will
return null! Which is an uncommen behaviour. Think about code like this:
> ObjectUtils.appendIdentityToString(buffer, param1)
>                     .appendIdentityToString(buffer, param2)
>                     .appendIdentityToString(buffer, param3);
> This will cause an NPE if param1 or param2 ist null. There may be other code where a
NPE will not happen, but the code is used for debugging and there will be an unexpected or
wrong output.
> So you shoul return the StringBuffer which is passed in or a new one if null. The harder
question is what to do with the object. I think we should append "null" to the StringBuffer,
because this is what I would expect and what the passed reference is.

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


Mime
View raw message