[
https://issues.apache.org/jira/browse/LANG-360?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539896
]
bayard edited comment on LANG-360 at 11/3/07 12:59 AM:
--------------------------------------------------------------
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 param1 - no param3. If you had 'buffer =' on the
front, then you just get param3 and no param1.
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?
was (Author: bayard):
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.
