You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by oh...@apache.org on 2005/08/19 18:16:48 UTC

svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src: java/org/apache/commons/configuration/ConfigurationKey.java test/org/apache/commons/configuration/TestConfigurationKey.java

Author: oheger
Date: Fri Aug 19 09:16:31 2005
New Revision: 233505

URL: http://svn.apache.org/viewcvs?rev=233505&view=rev
Log:
Minor fixes and increased unit test coverage

Modified:
    jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/ConfigurationKey.java
    jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestConfigurationKey.java

Modified: jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/ConfigurationKey.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/ConfigurationKey.java?rev=233505&r1=233504&r2=233505&view=diff
==============================================================================
--- jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/ConfigurationKey.java (original)
+++ jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/ConfigurationKey.java Fri Aug 19 09:16:31 2005
@@ -286,7 +286,7 @@
      */
     public int hashCode()
     {
-        return keyBuffer.hashCode();
+        return keyBuffer.toString().hashCode();
     }
 
     /**
@@ -548,7 +548,7 @@
         {
             boolean result = false;
 
-            int idx = key.indexOf(INDEX_START);
+            int idx = key.lastIndexOf(INDEX_START);
             if (idx > 0)
             {
                 int endidx = key.indexOf(INDEX_END, idx);

Modified: jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestConfigurationKey.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestConfigurationKey.java?rev=233505&r1=233504&r2=233505&view=diff
==============================================================================
--- jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestConfigurationKey.java (original)
+++ jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestConfigurationKey.java Fri Aug 19 09:16:31 2005
@@ -16,6 +16,8 @@
  * limitations under the License.
  */
 
+import java.util.NoSuchElementException;
+
 import junit.framework.TestCase;
 
 /**
@@ -59,6 +61,15 @@
         assertEquals("[@dataType]", it.currentKey(true));
         assertTrue(it.isAttribute());
         assertFalse(it.hasNext());
+        try
+        {
+            it.next();
+            fail("Could iterate over the iteration's end!");
+        }
+        catch(NoSuchElementException nex)
+        {
+            //ok
+        }
         
         key = new ConfigurationKey();
         assertFalse(key.iterator().hasNext());
@@ -66,6 +77,15 @@
         it = key.iterator();
         assertTrue(it.hasNext());
         assertEquals("simple", it.next());
+        try
+        {
+            it.remove();
+            fail("Could remove key component!");
+        }
+        catch(UnsupportedOperationException uex)
+        {
+            //ok
+        }
     }
     
     public void testAttribute()
@@ -103,6 +123,7 @@
         ConfigurationKey k2 = new ConfigurationKey(TESTKEY);
         assertTrue(k1.equals(k2));
         assertTrue(k2.equals(k1));
+        assertEquals(k1.hashCode(), k2.hashCode());
         k2.append("anotherPart");
         assertFalse(k1.equals(k2));
         assertFalse(k2.equals(k1));
@@ -132,6 +153,16 @@
         
         kc = k1.commonKey(k1);
         assertEquals(kc, k1);
+        
+        try
+        {
+            kc.commonKey(null);
+            fail("Could construct common key with null key!");
+        }
+        catch(IllegalArgumentException iex)
+        {
+            //ok
+        }
     }
     
     public void testDifferenceKey()
@@ -166,5 +197,29 @@
         assertEquals("trailing.dot.", kit.nextKey());
         assertEquals("strange", kit.nextKey());
         assertFalse(kit.hasNext());
+    }
+    
+    /**
+     * Tests some funny keys.
+     */
+    public void testIterateStrangeKeys()
+    {
+        ConfigurationKey k = new ConfigurationKey("key.");
+        ConfigurationKey.KeyIterator it = k.iterator();
+        assertTrue(it.hasNext());
+        assertEquals("key", it.next());
+        assertFalse(it.hasNext());
+        
+        k = new ConfigurationKey(".");
+        it = k.iterator();
+        assertFalse(it.hasNext());
+        
+        k = new ConfigurationKey("key().index()undefined(0).test");
+        it = k.iterator();
+        assertEquals("key()", it.next());
+        assertFalse(it.hasIndex());
+        assertEquals("index()undefined", it.nextKey(false));
+        assertTrue(it.hasIndex());
+        assertEquals(0, it.getIndex());
     }
 }



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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Oliver Heger <ol...@t-online.de> writes:

>In this case I agree with Brett. An uncought exception will be no 
>problem in a unit test. The fact that junit distinguishes between 
>failures and errors should especially direct the developer's attention 
>to that problem.

It's okay with me. I was just strolling through the commons-dev list
early in the morning with my coffee right besides me and pointing out
stuff. ;-)

	Regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

		      4 - 8 - 15 - 16 - 23 - 42

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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by Oliver Heger <ol...@t-online.de>.
In this case I agree with Brett. An uncought exception will be no 
problem in a unit test. The fact that junit distinguishes between 
failures and errors should especially direct the developer's attention 
to that problem.

Oliver

Brett Porter wrote:

>I'm a little lost on this criticism also... a test that doesn't catch an
>uncaught exception will error out. An error with a trace would be more
>obvious to me than the assertEquals below, and much easier to read.
>
>Henning P. Schmiedehausen wrote:
>
>  
>
>>oheger@apache.org writes:
>>
>> 
>>
>>    
>>
>>>+        try
>>>+        {
>>>+            it.next();
>>>+            fail("Could iterate over the iteration's end!");
>>>+        }
>>>+        catch(NoSuchElementException nex)
>>>+        {
>>>+            //ok
>>>+        }
>>>   
>>>
>>>      
>>>
>>This allows it.next() to throw another exception which leads to an
>>unchecked test failure. I found that using
>>
>>   try
>>   {
>>	it.next();
>>	fail("Could iterate over the iteration's end!");
>>   }
>>   catch(Exception e)
>>   {
>>	assertEquals("it.next() over end threw wrong exception", NoSuchElementException.class, e.getClass())
>>   }
>>
>>is more stable in the long run, because it makes sure that every
>>exception thrown by it.next() is caught.
>>
>>	Regards
>>		Henning
>>
>> 
>>
>>    
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>  
>


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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by Brett Porter <br...@apache.org>.
I'm a little lost on this criticism also... a test that doesn't catch an
uncaught exception will error out. An error with a trace would be more
obvious to me than the assertEquals below, and much easier to read.

Henning P. Schmiedehausen wrote:

>oheger@apache.org writes:
>
>  
>
>>+        try
>>+        {
>>+            it.next();
>>+            fail("Could iterate over the iteration's end!");
>>+        }
>>+        catch(NoSuchElementException nex)
>>+        {
>>+            //ok
>>+        }
>>    
>>
>
>This allows it.next() to throw another exception which leads to an
>unchecked test failure. I found that using
>
>    try
>    {
>	it.next();
>	fail("Could iterate over the iteration's end!");
>    }
>    catch(Exception e)
>    {
>	assertEquals("it.next() over end threw wrong exception", NoSuchElementException.class, e.getClass())
>    }
>
>is more stable in the long run, because it makes sure that every
>exception thrown by it.next() is caught.
>
>	Regards
>		Henning
>
>  
>


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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
oheger@apache.org writes:

>+        try
>+        {
>+            it.next();
>+            fail("Could iterate over the iteration's end!");
>+        }
>+        catch(NoSuchElementException nex)
>+        {
>+            //ok
>+        }

This allows it.next() to throw another exception which leads to an
unchecked test failure. I found that using

    try
    {
	it.next();
	fail("Could iterate over the iteration's end!");
    }
    catch(Exception e)
    {
	assertEquals("it.next() over end threw wrong exception", NoSuchElementException.class, e.getClass())
    }

is more stable in the long run, because it makes sure that every
exception thrown by it.next() is caught.

	Regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

		      4 - 8 - 15 - 16 - 23 - 42

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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by Oliver Heger <ol...@t-online.de>.
Henning P. Schmiedehausen wrote:

>Brett Porter <br...@apache.org> writes:
>
>  
>
>>How?
>>    
>>
>
>  
>
>>if keyBuffer is null (which it isn't, the constructor initialises it),
>>String.valueOf( keyBuffer ) would be an NPE as well. toString() should
>>not return null and won't for a StringBuffer.
>>    
>>
>
>What the...?????
>
>--- cut ----
>import java.io.File;
>
>public class test {
>    public static void main(String[] args) throws Exception {
>        System.out.println(String.valueOf((Object) null));
>    }
>}
>--- cut ----
>
>[henning@forge tmp]$ javac test.java
>[henning@forge tmp]$ java -version
>java version "1.5.0_02"
>Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_02-b09)
>Java HotSpot(TM) Client VM (build 1.5.0_02-b09, mixed mode, sharing)
>[henning@forge tmp]$ java test
>Exception in thread "main" java.lang.NullPointerException
>        at java.lang.String.<init>(String.java:173)
>        at java.lang.String.valueOf(String.java:2591)
>        at test.main(test.java:5)
>
>[henning@forge tmp]$ javac test.java
>[henning@forge tmp]$ java -version
>java version "1.4.2_08"
>Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_08-b03)
>Java HotSpot(TM) Client VM (build 1.4.2_08-b03, mixed mode)
>[henning@forge tmp]$ java test
>null
>
>Oh fsck, Sun, please, please, please don't tell me that you screwed
>_that_ one up.....
>
>The javadoc even states:
>
>http://java.sun.com/j2se/1.5.0/docs/api/java/lang/String.html#valueOf(java.lang.Object)
>
>--- cut ---
>valueOf
>
>public static String valueOf(Object obj)
>
>    Returns the string representation of the Object argument.
>
>    Parameters:
>        obj - an Object. 
>    Returns:
>        if the argument is null, then a string equal to "null";
>        otherwise, the value of obj.toString() is returned.
>    See Also:
>        Object.toString()
>
>--- cut ---
>
>Congrats. You found a bug. IMHO.
>
>This is BTW one of my personal mnemonics. 
>
>I do replace foobar.toString() with String.valueOf(foobar) because up
>until a few seconds ago I was under the firm impression, that
>String.valueOf(...) could _never_ throw NPE. This is a constant source
>of sorrow with code that I'm working on, because often co-workers tell
>me "that can never be null". Yeah, unless, some weird condition
>happens that was not tested for and surfaced after customer
>installation. Better safe than sorry. Defensive programming... :-)
>
>	Regards
>		Henning
>
>  
>
Despite of your test results I think that String.valueOf() is the safer 
variant. So I will change this.

Thanks for noticing.
Oliver

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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by sebb <se...@gmail.com>.
On 22/08/05, Henning P. Schmiedehausen <hp...@intermeta.de> wrote:
> Brett Porter <br...@apache.org> writes:
> 
> >How?
> 
> >if keyBuffer is null (which it isn't, the constructor initialises it),
> >String.valueOf( keyBuffer ) would be an NPE as well. toString() should
> >not return null and won't for a StringBuffer.
> 
> What the...?????
> 
> --- cut ----
> import java.io.File;
> 
> public class test {
>    public static void main(String[] args) throws Exception {
>        System.out.println(String.valueOf((Object) null));
>    }
> }
> --- cut ----
> 
> [henning@forge tmp]$ javac test.java
> [henning@forge tmp]$ java -version
> java version "1.5.0_02"
> Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_02-b09)
> Java HotSpot(TM) Client VM (build 1.5.0_02-b09, mixed mode, sharing)
> [henning@forge tmp]$ java test
> Exception in thread "main" java.lang.NullPointerException
>        at java.lang.String.<init>(String.java:173)
>        at java.lang.String.valueOf(String.java:2591)
>        at test.main(test.java:5)
> 

Seems to be fixed in the latest Windows JVM:

D:\temp>java -version
java version "1.5.0_04"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_04-b05)
Java HotSpot(TM) Client VM (build 1.5.0_04-b05, mixed mode, sharing)

D:\temp>java test
null

S.

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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Brett Porter <br...@apache.org> writes:

>How?

>if keyBuffer is null (which it isn't, the constructor initialises it),
>String.valueOf( keyBuffer ) would be an NPE as well. toString() should
>not return null and won't for a StringBuffer.

What the...?????

--- cut ----
import java.io.File;

public class test {
    public static void main(String[] args) throws Exception {
        System.out.println(String.valueOf((Object) null));
    }
}
--- cut ----

[henning@forge tmp]$ javac test.java
[henning@forge tmp]$ java -version
java version "1.5.0_02"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_02-b09)
Java HotSpot(TM) Client VM (build 1.5.0_02-b09, mixed mode, sharing)
[henning@forge tmp]$ java test
Exception in thread "main" java.lang.NullPointerException
        at java.lang.String.<init>(String.java:173)
        at java.lang.String.valueOf(String.java:2591)
        at test.main(test.java:5)

[henning@forge tmp]$ javac test.java
[henning@forge tmp]$ java -version
java version "1.4.2_08"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_08-b03)
Java HotSpot(TM) Client VM (build 1.4.2_08-b03, mixed mode)
[henning@forge tmp]$ java test
null

Oh fsck, Sun, please, please, please don't tell me that you screwed
_that_ one up.....

The javadoc even states:

http://java.sun.com/j2se/1.5.0/docs/api/java/lang/String.html#valueOf(java.lang.Object)

--- cut ---
valueOf

public static String valueOf(Object obj)

    Returns the string representation of the Object argument.

    Parameters:
        obj - an Object. 
    Returns:
        if the argument is null, then a string equal to "null";
        otherwise, the value of obj.toString() is returned.
    See Also:
        Object.toString()

--- cut ---

Congrats. You found a bug. IMHO.

This is BTW one of my personal mnemonics. 

I do replace foobar.toString() with String.valueOf(foobar) because up
until a few seconds ago I was under the firm impression, that
String.valueOf(...) could _never_ throw NPE. This is a constant source
of sorrow with code that I'm working on, because often co-workers tell
me "that can never be null". Yeah, unless, some weird condition
happens that was not tested for and surfaced after customer
installation. Better safe than sorry. Defensive programming... :-)

	Regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

		      4 - 8 - 15 - 16 - 23 - 42

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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by Brett Porter <br...@apache.org>.
How?

if keyBuffer is null (which it isn't, the constructor initialises it),
String.valueOf( keyBuffer ) would be an NPE as well. toString() should
not return null and won't for a StringBuffer.

- Brett

Henning P. Schmiedehausen wrote:

>oheger@apache.org writes:
>
>  
>
>>    public int hashCode()
>>    {
>>-        return keyBuffer.hashCode();
>>+        return keyBuffer.toString().hashCode();
>>    }
>>    
>>
>
>This screams "NullPointerException" all over the place.
>
>How about
>
>public int hashCode() {
>	return String.valueOf(keyBuffer).hashCode();
>}
>
>	Regards
>		Henning
>
>
>  
>


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


Re: svn commit: r233505 - in /jakarta/commons/proper/configuration/trunk/src:

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
oheger@apache.org writes:

>     public int hashCode()
>     {
>-        return keyBuffer.hashCode();
>+        return keyBuffer.toString().hashCode();
>     }

This screams "NullPointerException" all over the place.

How about

public int hashCode() {
	return String.valueOf(keyBuffer).hashCode();
}

	Regards
		Henning


-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

		      4 - 8 - 15 - 16 - 23 - 42

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