Skip to content

[SPARK-13702][CORE][SQL][MLLIB] Use diamond operator for generic instance creation in Java code. #11541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
dongjoon-hyun wants to merge 2 commits into from
Closed

[SPARK-13702][CORE][SQL][MLLIB] Use diamond operator for generic instance creation in Java code. #11541

dongjoon-hyun wants to merge 2 commits into from

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Mar 5, 2016

What changes were proposed in this pull request?

In order to make docs/examples (and other related code) more simple/readable/user-friendly, this PR replaces existing codes like the followings by using diamond operator.

-    final ArrayList<Product2<Object, Object>> dataToWrite =
-      new ArrayList<Product2<Object, Object>>();
+    final ArrayList<Product2<Object, Object>> dataToWrite = new ArrayList<>();

Java 7 or higher supports diamond operator which replaces the type arguments required to invoke the constructor of a generic class with an empty set of type parameters (<>). Currently, Spark Java code use mixed usage of this.

How was this patch tested?

Manual.
Pass the existing tests.

@srowen
Copy link
Member

srowen commented Mar 6, 2016

Although I personally like this change, I'm less certain about it since it does not add any functional improvement or avoid a potential bug. It does add a little readability and only touches the relatively fewer Java files. I'd like to wait for others to give an opinion.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 6, 2016

Thank you for review, @srowen .
I agree. This has a little limited benefit; only small portion Java and no functional improvement.
Actually, I started this PR to improve docs/examples (28 of 61 files) for simplicity and eventually hoped to apply all code to keep it consistently.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 8, 2016

The conflict will be resolved via #11567 . After #11567 is committed, I will update this.

@mengxr
Copy link
Contributor

mengxr commented Mar 8, 2016

+1 on the code changes, which simplifies the Java code and shows some benefits from dropping Java 6, but the changes to the pom file should be discussed in a separate PR.

@mengxr
Copy link
Contributor

mengxr commented Mar 8, 2016

add to whitelist

@mengxr
Copy link
Contributor

mengxr commented Mar 8, 2016

ok to test

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 8, 2016

Thank you, @mengxr !
I will remove pom.xml and checkstyle part from this PR soon.

@dongjoon-hyun
[SPARK-13702][CORE][SQL][MLLIB] Use diamond operator for generic inst…
5d090a0 
…ance creation in Java code.

Java 7 or higher supports `diamond` operator which replaces the type arguments required to invoke the constructor of a generic class with an empty set of type parameters (<>). Currently, Spark Java code use mixed usage of this. This issue replaces existing codes to use `diamond` operator and add Checkstyle rule.
```
-List<JavaPairDStream<String, String>> kafkaStreams = new ArrayList<JavaPairDStream<String, String>>(numStreams);
+List<JavaPairDStream<String, String>> kafkaStreams = new ArrayList<>(numStreams);
-Set<Tuple2<Integer, Integer>> edges = new HashSet<Tuple2<Integer, Integer>>(numEdges);
+Set<Tuple2<Integer, Integer>> edges = new HashSet<>(numEdges);
```
@dongjoon-hyun
Remove Checkstyle enforcement
2e20843
@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 8, 2016

Hi, @mengxr .
I removed the change on build related files. Now this PR contains pure Java and md files.
Also, the description of PR/JIRA issue is updated accordingly.

@SparkQA
Copy link

SparkQA commented Mar 9, 2016

Test build #52697 has finished for PR 11541 at commit 78e3d32.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 9, 2016

Test build #52699 has finished for PR 11541 at commit 2e20843.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 9, 2016

Hi, @mengxr .
The latest commit passes the test.
Could you review this again please?

@SparkQA
Copy link

SparkQA commented Mar 9, 2016

Test build #52696 has finished for PR 11541 at commit 78e3d32.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 9, 2016

The latest one is at commit 2e20843, and all three builds (including that) passes.

@srowen
Copy link
Member

srowen commented Mar 9, 2016

LGTM, merging to master. This is now just straightforward use of the diamond operator only.

@asfgit asfgit closed this in c3689bc Mar 9, 2016
@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 9, 2016

Thank you, @srowen . Yes, it is.

roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
@dongjoon-hyun @roygao94
[SPARK-13702][CORE][SQL][MLLIB] Use diamond operator for generic inst…
876091a 
…ance creation in Java code.

## What changes were proposed in this pull request?

In order to make `docs/examples` (and other related code) more simple/readable/user-friendly, this PR replaces existing codes like the followings by using `diamond` operator.

```
-    final ArrayList<Product2<Object, Object>> dataToWrite =
-      new ArrayList<Product2<Object, Object>>();
+    final ArrayList<Product2<Object, Object>> dataToWrite = new ArrayList<>();
```

Java 7 or higher supports **diamond** operator which replaces the type arguments required to invoke the constructor of a generic class with an empty set of type parameters (<>). Currently, Spark Java code use mixed usage of this.

## How was this patch tested?

Manual.
Pass the existing tests.

Author: Dongjoon Hyun <[email protected]>

Closes apache#11541 from dongjoon-hyun/SPARK-13702.
@dongjoon-hyun dongjoon-hyun deleted the SPARK-13702 branch March 29, 2016 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dongjoon-hyun @srowen @mengxr @SparkQA