[ 
https://issues.apache.org/jira/browse/DRILL-6613?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16547402#comment-16547402
 ] 

ASF GitHub Bot commented on DRILL-6613:
---------------------------------------

paul-rogers commented on a change in pull request #1383: DRILL-6613: Refactor 
MaterializedField
URL: https://github.com/apache/drill/pull/1383#discussion_r203253140
 
 

 ##########
 File path: 
exec/java-exec/src/test/java/org/apache/drill/exec/record/TestMaterializedField.java
 ##########
 @@ -58,32 +58,24 @@ public void initialize() {
   }
 
   @Test
-  public void testClone() {
-    final MaterializedField cloneParent = parent.clone();
-    final boolean isParentEqual = parent.equals(cloneParent);
-    assertTrue("Cloned parent does not match the original", isParentEqual);
+  public void testCopy() {
+    final MaterializedField cloneParent = parent.copy();
+    assertEquals("Parent copy does not match the original", parent, 
cloneParent);
 
-    final MaterializedField cloneChild = child.clone();
-    final boolean isChildEqual = child.equals(cloneChild);
-    assertTrue("Cloned child does not match the original", isChildEqual);
+    final MaterializedField cloneChild = child.copy();
+    assertEquals("Child copy does not match the original", child, cloneChild);
 
-    for (final MaterializedField field:new MaterializedField[]{parent, child}) 
{
-      for (Object[] args:matrix) {
-        final String path = args[0].toString();
-        final TypeProtos.MajorType type = 
TypeProtos.MajorType.class.cast(args[1]);
-
-        final MaterializedField clone = field.withPathAndType(path, type);
-
-        final boolean isPathEqual = path.equals(clone.getName());
-        assertTrue("Cloned path does not match the original", isPathEqual);
-
-        final boolean isTypeEqual = type.equals(clone.getType());
-        assertTrue("Cloned type does not match the original", isTypeEqual);
-
-        final boolean isChildrenEqual = 
field.getChildren().equals(clone.getChildren());
-        assertTrue("Cloned children do not match the original", 
isChildrenEqual);
-      }
+    for (Object[] args : matrix) {
+      assertTypeAndPath(parent, (String)args[0], 
(TypeProtos.MajorType)args[1]);
+      assertTypeAndPath(child, (String)args[0], (TypeProtos.MajorType)args[1]);
     }
+  }
+
+  private void assertTypeAndPath(MaterializedField field, String path, 
TypeProtos.MajorType type) {
+    final MaterializedField clone = field.copy(path, type);
 
 Review comment:
   What are we copying if we change the name and the type? Are we not just 
keeping the mode? Seems odd... When would we do that? Isn't there a cleaner way 
to express that idea?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Refactor MaterializedField
> --------------------------
>
>                 Key: DRILL-6613
>                 URL: https://issues.apache.org/jira/browse/DRILL-6613
>             Project: Apache Drill
>          Issue Type: Improvement
>            Reporter: Vlad Rozov
>            Assignee: Vlad Rozov
>            Priority: Minor
>
> {{MaterializedField}} does not need to implement {{clone()}} and should use 
> constructor.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to