[jira] [Commented] (IMPALA-7867) Expose collection interfaces, not implementations
[
https://issues.apache.org/jira/browse/IMPALA-7867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16758591#comment-16758591
]
ASF subversion and git services commented on IMPALA-7867:
-
Commit 396f542eda32dd92e80edbeb216a4cdeb7fe0ace in impala's branch
refs/heads/master from paul-rogers
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=396f542 ]
IMPALA-7867 (Part 5): Collection cleanup in analyzer
Continues the work to clean up the code to:
* Use collection interfaces for variable and function declarations,
* Replace Guava newArrayList(), etc. calls with the direct
use of Java collection classes.
* Clean up unused imports and add override annotations.
This commit cleans up remaining issues in the analyzer now that the
other modules use collection interfaces.
Tests: this is purely a refactoring with no functional change. Reran
existing tests.
Change-Id: I1d1c37beb926896f5e00faab0b06034aebb835c5
Reviewed-on: http://gerrit.cloudera.org:8080/12266
Reviewed-by: Impala Public Jenkins
Tested-by: Impala Public Jenkins
> Expose collection interfaces, not implementations
> -
>
> Key: IMPALA-7867
> URL: https://issues.apache.org/jira/browse/IMPALA-7867
> Project: IMPALA
> Issue Type: Improvement
> Components: Frontend
>Affects Versions: Impala 3.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>
> When using Java collections, a common Java best practice is to expose the
> collection interface, but hide the implementation choice. This pattern allows
> us to start with a generic implementation (an {{ArrayList}}, say), but evolve
> to a more specific implementation to achieve certain goals (a {{LinkedList}}
> or {{ImmutableList}}, say.)
> For whatever reason, the Impala FE code exposes {{ArrayList}}, {{HashMap}}
> and other implementation choices as variable types and in method signatures.
> This ticket tracks a gradual process of revising the declarations and
> signatures to use the interfaces {{List}} instead of the implementation
> {{ArrayList}}.
> Also, the FE code appears to predate Java 7, so that declarations of lists
> tend to be in one of two forms (with or without Guava):
> {code:java}
> foo1 = new ArrayList();
> foo2 = Lists.newArrayList();
> {code}
> Since Java 7, the preferred form is:
> {code:java}
> foo = new ArrayList<>();
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
[jira] [Commented] (IMPALA-7867) Expose collection interfaces, not implementations
[
https://issues.apache.org/jira/browse/IMPALA-7867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16739786#comment-16739786
]
ASF subversion and git services commented on IMPALA-7867:
-
Commit 049e1056f8d772c55274a54ff99e2cf44b82fa56 in impala's branch
refs/heads/master from paul-rogers
[ https://git-wip-us.apache.org/repos/asf?p=impala.git;h=049e105 ]
IMPALA-7867 (Part 4): Collection cleanup in catalog
Continues the collection clean work to:
* Use collection interfaces for variable and function argument
declarations,
* Replace generic Guava newArrayList(), etc. calls with the direct use
of the Java collection classes,
* Clean up unused imports and add override annotations.
This patch focuses on the catalog module and its tests.
Tests: this is purely a code change, no functional change. Reran
existing tests.
Change-Id: Ic83425201c90966aae4c280d94cf1b427b3d71d1
Reviewed-on: http://gerrit.cloudera.org:8080/12131
Reviewed-by: Impala Public Jenkins
Tested-by: Impala Public Jenkins
> Expose collection interfaces, not implementations
> -
>
> Key: IMPALA-7867
> URL: https://issues.apache.org/jira/browse/IMPALA-7867
> Project: IMPALA
> Issue Type: Improvement
> Components: Frontend
>Affects Versions: Impala 3.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>
> When using Java collections, a common Java best practice is to expose the
> collection interface, but hide the implementation choice. This pattern allows
> us to start with a generic implementation (an {{ArrayList}}, say), but evolve
> to a more specific implementation to achieve certain goals (a {{LinkedList}}
> or {{ImmutableList}}, say.)
> For whatever reason, the Impala FE code exposes {{ArrayList}}, {{HashMap}}
> and other implementation choices as variable types and in method signatures.
> This ticket tracks a gradual process of revising the declarations and
> signatures to use the interfaces {{List}} instead of the implementation
> {{ArrayList}}.
> Also, the FE code appears to predate Java 7, so that declarations of lists
> tend to be in one of two forms (with or without Guava):
> {code:java}
> foo1 = new ArrayList();
> foo2 = Lists.newArrayList();
> {code}
> Since Java 7, the preferred form is:
> {code:java}
> foo = new ArrayList<>();
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
[jira] [Commented] (IMPALA-7867) Expose collection interfaces, not implementations
[
https://issues.apache.org/jira/browse/IMPALA-7867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726992#comment-16726992
]
ASF subversion and git services commented on IMPALA-7867:
-
Commit 32fc07b50c24496510eaf280573c75aac7d43f99 in impala's branch
refs/heads/master from Paul Rogers
[ https://git-wip-us.apache.org/repos/asf?p=impala.git;h=32fc07b ]
IMPALA-7867 (Part 3): ArrayList cleanup in planner
Continues the work to tidy up uses of ArrayList, HashMap and HashSet,
this time in the planner package and its dependencies. Changed the code
to follow standard Java practice: use base interfaces for method and
variable declarations, specific subclasses when creating objects.
Replaced trivial uses of Guava newFoo() methods with direct object
creation such as new ArrayList<>() as recommended by Guava. Fixed
trivial warnings such as unused imports and missing @Override. Removed
constuctor type parameters when the compiler can infer them.
Testing: this is a pure source change, no functional changes. Ran the
pre-review tests to verify no regressions.
Change-Id: I9597428c69173b43fbc3cb26027ad257f2d8fccb
Reviewed-on: http://gerrit.cloudera.org:8080/12094
Reviewed-by: Impala Public Jenkins
Tested-by: Impala Public Jenkins
> Expose collection interfaces, not implementations
> -
>
> Key: IMPALA-7867
> URL: https://issues.apache.org/jira/browse/IMPALA-7867
> Project: IMPALA
> Issue Type: Improvement
> Components: Frontend
>Affects Versions: Impala 3.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>
> When using Java collections, a common Java best practice is to expose the
> collection interface, but hide the implementation choice. This pattern allows
> us to start with a generic implementation (an {{ArrayList}}, say), but evolve
> to a more specific implementation to achieve certain goals (a {{LinkedList}}
> or {{ImmutableList}}, say.)
> For whatever reason, the Impala FE code exposes {{ArrayList}}, {{HashMap}}
> and other implementation choices as variable types and in method signatures.
> This ticket tracks a gradual process of revising the declarations and
> signatures to use the interfaces {{List}} instead of the implementation
> {{ArrayList}}.
> Also, the FE code appears to predate Java 7, so that declarations of lists
> tend to be in one of two forms (with or without Guava):
> {code:java}
> foo1 = new ArrayList();
> foo2 = Lists.newArrayList();
> {code}
> Since Java 7, the preferred form is:
> {code:java}
> foo = new ArrayList<>();
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
[jira] [Commented] (IMPALA-7867) Expose collection interfaces, not implementations
[
https://issues.apache.org/jira/browse/IMPALA-7867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16721615#comment-16721615
]
ASF subversion and git services commented on IMPALA-7867:
-
Commit 520a2b150cade5b83611bb11abea7997cf535cdc in impala's branch
refs/heads/master from [~paul-rogers]
[ https://git-wip-us.apache.org/repos/asf?p=impala.git;h=520a2b1 ]
IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Follow-on to prior patch for this JIRA. Replaces all use of ArrayList
in variable and method declarations with the interface List. Retains
the use of ArrayList for list implementations (i.e. "new" statements.)
Also replaces "List.newArrayList()" with the more modern
"new ArrayList<>()". Cleaned up a few Map and Set declarations and
added a few missing @Override annotations found during this process.
Similar changes made for HashMap/newHashMap() and HashSet/newHashSet().
Where I noticed old-style new calls (List = new ArrayList())
these were replaced with diamond-notation: new ArrayList<>().
Tests: This is purely a source-level change; no functional change. Ran
all client unit tests.
Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a
Reviewed-on: http://gerrit.cloudera.org:8080/11995
Reviewed-by: Fredy Wijaya
Tested-by: Impala Public Jenkins
> Expose collection interfaces, not implementations
> -
>
> Key: IMPALA-7867
> URL: https://issues.apache.org/jira/browse/IMPALA-7867
> Project: IMPALA
> Issue Type: Improvement
> Components: Frontend
>Affects Versions: Impala 3.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>
> When using Java collections, a common Java best practice is to expose the
> collection interface, but hide the implementation choice. This pattern allows
> us to start with a generic implementation (an {{ArrayList}}, say), but evolve
> to a more specific implementation to achieve certain goals (a {{LinkedList}}
> or {{ImmutableList}}, say.)
> For whatever reason, the Impala FE code exposes {{ArrayList}}, {{HashMap}}
> and other implementation choices as variable types and in method signatures.
> This ticket tracks a gradual process of revising the declarations and
> signatures to use the interfaces {{List}} instead of the implementation
> {{ArrayList}}.
> Also, the FE code appears to predate Java 7, so that declarations of lists
> tend to be in one of two forms (with or without Guava):
> {code:java}
> foo1 = new ArrayList();
> foo2 = Lists.newArrayList();
> {code}
> Since Java 7, the preferred form is:
> {code:java}
> foo = new ArrayList<>();
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
[jira] [Commented] (IMPALA-7867) Expose collection interfaces, not implementations
[
https://issues.apache.org/jira/browse/IMPALA-7867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16699777#comment-16699777
]
ASF subversion and git services commented on IMPALA-7867:
-
Commit 12dc29e5ee1ec03bbc1556eca4539645e2b9bbf1 in impala's branch
refs/heads/master from [~paul-rogers]
[ https://git-wip-us.apache.org/repos/asf?p=impala.git;h=12dc29e ]
IMPALA-7867 (Part 1): Expose List in TreeNode, parser
When using Java collections, a common Java best practice is to expose
the collection interface, but hide the implementation choice. This
pattern allows us to start with a generic implementation (an ArrayList,
say), but evolve to a more specific implementation to achieve certain
goals (a LinkedList or ImmutableList, say.)
For whatever reason, the Impala FE code exposes ArrayList, HashMap and
other implementation choices as variable types and in method signatures.
Also, since Java 7, the preferred way to create an array is
new ArrayList<>()
Replaced older forms:
new ArrayList() // Pre-Java 7
Lists.newArrayList() // Guava form, pre-Java 7
This ticket cleans up two files, and their dependencies:
* TreeNode (the root of all parser nodes)
* sql-parser.cup (the code which creates the parser nodes)
Many other uses exist, and will be submitted as separate patches to keep
patches small.
In TreeNode, also cleaned up some of the generic expresions, which
caused dependencies to change in order to be more type-safe.
Tests: This is purely a refactoring, no functionality changed. Ran the
FE unit tests to verify no regressions.
Change-Id: Iebab7dccdb4b2fa0b5ca812beab0e8bdba39f539
Reviewed-on: http://gerrit.cloudera.org:8080/11954
Reviewed-by: Impala Public Jenkins
Tested-by: Impala Public Jenkins
> Expose collection interfaces, not implementations
> -
>
> Key: IMPALA-7867
> URL: https://issues.apache.org/jira/browse/IMPALA-7867
> Project: IMPALA
> Issue Type: Improvement
> Components: Frontend
>Affects Versions: Impala 3.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>
> When using Java collections, a common Java best practice is to expose the
> collection interface, but hide the implementation choice. This pattern allows
> us to start with a generic implementation (an {{ArrayList}}, say), but evolve
> to a more specific implementation to achieve certain goals (a {{LinkedList}}
> or {{ImmutableList}}, say.)
> For whatever reason, the Impala FE code exposes {{ArrayList}}, {{HashMap}}
> and other implementation choices as variable types and in method signatures.
> This ticket tracks a gradual process of revising the declarations and
> signatures to use the interfaces {{List}} instead of the implementation
> {{ArrayList}}.
> Also, the FE code appears to predate Java 7, so that declarations of lists
> tend to be in one of two forms (with or without Guava):
> {code:java}
> foo1 = new ArrayList();
> foo2 = Lists.newArrayList();
> {code}
> Since Java 7, the preferred form is:
> {code:java}
> foo = new ArrayList<>();
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
[jira] [Commented] (IMPALA-7867) Expose collection interfaces, not implementations
[
https://issues.apache.org/jira/browse/IMPALA-7867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16691195#comment-16691195
]
Paul Rogers commented on IMPALA-7867:
-
[~marcelk], thanks much for the note. The guarantees you site for {{ArrayList}}
are accurate. This is why, when _creating_ a list, {{ArrayList}} is often a
good choice.
However, when _declaring_ a list, the interface is sufficient. Though a
variable is declared as {{List}}, its implementation is actually {{ArrayList}}
if declared that way.
Not sure if this is a difference between Java and C++ behavior, but it is
pretty standard Java practice to hind the implementation choice and expose only
the generic interface.
Example:
{code:java}
public class Foo {
// Declaration is generic, implementation is ArrayList
private final List myList_ = new ArrayList<>();
// Interface is generic
List getList() { return myList_; }
// Implementation in terms of generic interface, implementation
// is O(1) as guaranteed by ArrayList.
void Bar getBar(int i) {
return myList_.get(i);
}
}
{code}
Does this clear up the confusion?
> Expose collection interfaces, not implementations
> -
>
> Key: IMPALA-7867
> URL: https://issues.apache.org/jira/browse/IMPALA-7867
> Project: IMPALA
> Issue Type: Improvement
> Components: Frontend
>Affects Versions: Impala 3.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>
> When using Java collections, a common Java best practice is to expose the
> collection interface, but hide the implementation choice. This pattern allows
> us to start with a generic implementation (an {{ArrayList}}, say), but evolve
> to a more specific implementation to achieve certain goals (a {{LinkedList}}
> or {{ImmutableList}}, say.)
> For whatever reason, the Impala FE code exposes {{ArrayList}}, {{HashMap}}
> and other implementation choices as variable types and in method signatures.
> This ticket tracks a gradual process of revising the declarations and
> signatures to use the interfaces {{List}} instead of the implementation
> {{ArrayList}}.
> Also, the FE code appears to predate Java 7, so that declarations of lists
> tend to be in one of two forms (with or without Guava):
> {code:java}
> foo1 = new ArrayList();
> foo2 = Lists.newArrayList();
> {code}
> Since Java 7, the preferred form is:
> {code:java}
> foo = new ArrayList<>();
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
