Code review: change so that char -> String, char[] -> List<String>

neo4j · Jul 5, 2016 · 337fe5b · 337fe5b
1 parent cb56f04
commit 337fe5b
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 15 deletions.
diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPack.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPack.java
@@ -66,6 +66,7 @@ public Packer( PackOutput output )
             super( output );
         }
 
+        @SuppressWarnings( "unchecked" )
         public void pack( Object obj ) throws IOException
         {
             // Note: below uses instanceof for quick implementation, this should be swapped over
@@ -96,8 +97,7 @@ else if ( obj instanceof String )
             }
             else if (obj instanceof Character )
             {
-                Character character = (Character) obj;
-                pack( character.toString() );
+                pack( (Character) obj );
             }
             else if ( obj instanceof Map )
             {
@@ -127,8 +127,12 @@ else if ( obj instanceof byte[] )
             }
             else if ( obj instanceof char[] )
             {
-                // Treat it as a String
-                pack( new String( (char[]) obj ) );
+                char[] array = (char[]) obj;
+                packListHeader( array.length );
+                for ( char item : array )
+                {
+                    pack( item );
+                }
             }
             else if ( obj instanceof short[] )
             {

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/packstream/PackStream.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/packstream/PackStream.java
@@ -221,6 +221,11 @@ public void pack( char character ) throws IOException
             pack( new String( new char[]{character} ) );
         }
 
+        public void pack( Character character ) throws IOException
+        {
+            pack( new String( new char[]{character} ) );
+        }
+
         public void pack( String value ) throws IOException
         {
             if ( value == null ) { packNull(); }

diff --git a/community/bolt/src/test/java/org/neo4j/bolt/v1/messaging/Neo4jPackTest.java b/community/bolt/src/test/java/org/neo4j/bolt/v1/messaging/Neo4jPackTest.java
@@ -36,6 +36,7 @@
 import org.neo4j.graphdb.Path;
 import org.neo4j.kernel.impl.util.HexPrinter;
 
+import static java.util.Arrays.asList;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -68,6 +69,7 @@ private Object unpacked( byte[] bytes ) throws IOException
         return unpacker.unpack();
     }
 
+    @SuppressWarnings( "unchecked" )
     @Test
     public void shouldBeAbleToPackAndUnpackListStream() throws IOException
     {
@@ -91,14 +93,15 @@ public void shouldBeAbleToPackAndUnpackListStream() throws IOException
         assertThat( unpackedList, equalTo( expected ) );
     }
 
+    @SuppressWarnings( "unchecked" )
     @Test
     public void shouldBeAbleToPackAndUnpackMapStream() throws IOException
     {
         // Given
         PackedOutputArray output = new PackedOutputArray();
         Neo4jPack.Packer packer = new Neo4jPack.Packer( output );
         packer.packMapStreamHeader();
-        for ( Map.Entry<String, Object> entry : ALICE.getAllProperties().entrySet() )
+        for ( Map.Entry<String,Object> entry : ALICE.getAllProperties().entrySet() )
         {
             packer.pack( entry.getKey() );
             packer.pack( entry.getValue() );
@@ -108,17 +111,18 @@ public void shouldBeAbleToPackAndUnpackMapStream() throws IOException
 
         // Then
         assertThat( unpacked, instanceOf( Map.class ) );
-        Map<String, Object> unpackedMap = (Map<String, Object>) unpacked;
+        Map<String,Object> unpackedMap = (Map<String,Object>) unpacked;
         assertThat( unpackedMap, equalTo( ALICE.getAllProperties() ) );
     }
 
+    @SuppressWarnings( "unchecked" )
     @Test
     public void shouldFailNicelyWhenPackingAMapWithUnpackableValues() throws IOException
     {
         // Given
         PackedOutputArray output = new PackedOutputArray();
         Neo4jPack.Packer packer = new Neo4jPack.Packer( output );
-        packer.packRawMap( map("unpackable", new Unpackable() ) );
+        packer.packRawMap( map( "unpackable", new Unpackable() ) );
         Object unpacked = unpacked( output.bytes() );
 
         // Then
@@ -151,10 +155,10 @@ public void shouldErrorOnUnpackingMapWithDuplicateKeys() throws IOException
     public void shouldHandleDeletedNodesGracefully() throws IOException
     {
         // Given
-        Node node = mock(Node.class);
-        when(node.getId()).thenReturn( 42L );
-        doThrow( NotFoundException.class ).when(node).getAllProperties(  );
-        doThrow( NotFoundException.class ).when(node).getLabels(  );
+        Node node = mock( Node.class );
+        when( node.getId() ).thenReturn( 42L );
+        doThrow( NotFoundException.class ).when( node ).getAllProperties();
+        doThrow( NotFoundException.class ).when( node ).getLabels();
 
         // When
         byte[] packed = packed( node );
@@ -166,7 +170,7 @@ public void shouldHandleDeletedNodesGracefully() throws IOException
         //    labels: [] (90)
         //    props: {} (A0)
         //}
-        assertThat(HexPrinter.hex( packed ), equalTo("B3 4E 2A 90 A0"));
+        assertThat( HexPrinter.hex( packed ), equalTo( "B3 4E 2A 90 A0" ) );
     }
 
     @Test
@@ -214,7 +218,7 @@ public void shouldTreatSingleCharAsSingleCharacterString() throws IOException
     }
 
     @Test
-    public void shouldTreatCharArrayAsString() throws IOException
+    public void shouldTreatCharArrayAsListOfStrings() throws IOException
     {
         // Given
         PackedOutputArray output = new PackedOutputArray();
@@ -223,8 +227,8 @@ public void shouldTreatCharArrayAsString() throws IOException
         Object unpacked = unpacked( output.bytes() );
 
         // Then
-        assertThat( unpacked, instanceOf( String.class ) );
-        assertThat( unpacked, equalTo( "WHY" ) );
+        assertThat( unpacked, instanceOf( List.class ) );
+        assertThat( unpacked, equalTo( asList( "W", "H", "Y" ) ) );
     }
 
     private static class Unpackable