Hi Alan,
On 2/11/2011 7:47 PM, Alan Bateman wrote:
On 01/11/2011 23:54, David Holmes wrote:
This fix seems inconsistent with how this non-String problem is
handled elsewhere.
It is, but (for whatever reason) is specified to return "the previous
string value of the system property" whereas Properties.getProperty is
specified to return "the value in this property list with the specified
key value".
I'm not seeing a distinction in those two statements. Both expect to
return the property value for a given key; both assume a valid value is
a String. clearProperty throws ClassCastException if the assumption
doesn't hold; getProperty instead returns null.
Given that System.clearProperty has always been broken then
there may be an opportunity to change it so that it is consistent and
returns null.
I'm not even sure this is broken. If you put something other than a
String in a Property then you get what you deserve. ClassCastException
seems quite reasonable to me.
But if we must get rid of the exception then getProperty has established
the pattern. I don't think calling toString on an arbitrary object is
the right way to deal with this.
I also think Properties.getProperty could be clearer as it
doesn't appear to specify that it returns null when the value isn't of
type String.
There's an assumption that everyone reads and obeys the directive to not
use the Hashtable methods that allow you to insert non-strings.
Cheers,
David
-----
Darryl - just a couple of comments on the test - as it doesn't clean up
after itself then it will need to run in its own VM to ensure that it
doesn't cause problems for other tests that run subsequently in the same
VM (@run main/othervm ClearProperty). Another nit with the test is that
it doesn't check the return value of System.clearProperty. Another
comment is that it doesn't need to catch CCE as the test will fail
anyway if thrown. Also it would be nice to separate the test comment
from the comment block with the jtreg tags so that it's consistent with
other tests (alternatively just expand the wording in the @summary tag).
-Alan.
