Skip to content
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.

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

ZOOKEEPER-2630: Use interface type instead of implementation type whe… #354

Closed
wants to merge 2 commits into from

Conversation

tamaashu
Copy link
Contributor

Use interface type instead of implementation type when appropriate.

There are a couple of places in code base where we declare a field / variable as implementation type (i.e. HashMap, HashSet) instead of interface type (i.e. Map, Set), while in other places we do the opposite by declaring as interface type. A quick check indicates that most if not all of these places could be updated so we have a consistent style over the code base (prefer using interface type), which is also a good coding style to stick per best practice.

Checked and fixed Set, Map and List interface usages.

…n appropriate.

Checked and fixed Set, Map and List interface usages.
Copy link
Contributor

@afine afine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I checked this using the jar compatibility script added in ZOOKEEPER-2864 and everything looks great. I was just wondering if you used a tool to make all these changes or did so manually.

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we generally avoid using * inputs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working on the fix in my next commit

@tamaashu
Copy link
Contributor Author

tamaashu commented Sep 1, 2017

fixed the issues with the imports

Copy link
Contributor

@hanm hanm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work @tamaashu and thanks for additional verification work of @afine using the newly added API compatibility checker tool!

@asfgit asfgit closed this in 1165794 Sep 11, 2017
@hanm
Copy link
Contributor

hanm commented Sep 11, 2017

Committed to master: 1165794.

@hanm
Copy link
Contributor

hanm commented Sep 11, 2017

@tamaashu This patch does not apply to branch-3.4 and branch-3.5. For now it's only applied on master and I resolved the JIRA with a target version of 3.6. If you'd like this land on 3.4 and 3.5, please create separate pull requests.

@eribeiro
Copy link
Contributor

Late to the party but awesome work! Congrats! 👍

@hanm are those changes applied to both master and branch-3.5? No branch-4.5 backport, right?

@hanm
Copy link
Contributor

hanm commented Sep 11, 2017

@eribeiro not sure why you asked - thought I made it clear that the patch for now only applied on master, because this pull request can't apply as is on other branches without some additional work which I don't have time to do myself. That said, if you or anyone else want same change land on other branches, please feel free to propose pull requests.

@eribeiro
Copy link
Contributor

@hanm oh, totally my fault asking an already answered question. Excuse me. Even tough, I see my (useless) question and your comment were roughly 9 hours ago, and I remember very well the last comment I saw on this thread being "Committed to master: 1165794." (also about 9 hours ago). So, I suppose it was some sort of eventual consistency glitch that prevented me from reading your comment before posting. Whatever, sorry about that.

@tamaashu Please, let me know if you are planning to work on porting the changes in this PR to branch-3.5 and branch-3.4. If you cannot and don't mind, I can pick up the work of porting the changes in this PR to branch-3.5, at least. Best regards!

tamaashu added a commit to tamaashu/zookeeper that referenced this pull request Oct 6, 2017
Use interface type instead of implementation type when appropriate.

There are a couple of places in code base where we declare a field / variable as implementation type (i.e. HashMap, HashSet) instead of interface type (i.e. Map, Set), while in other places we do the opposite by declaring as interface type. A quick check indicates that most if not all of these places could be updated so we have a consistent style over the code base (prefer using interface type), which is also a good coding style to stick per best practice.

Checked and fixed Set, Map and List interface usages.

Author: Tamas Penzes <tamaas@cloudera.com>

Reviewers: Abe Fine <afine@apache.org>, Michael Han <hanm@apache.org>

Closes apache#354 from tamaashu/ZOOKEEPER-2630

(cherry picked from commit 1165794)
tamaashu added a commit to tamaashu/zookeeper that referenced this pull request Oct 9, 2017
Use interface type instead of implementation type when appropriate.

There are a couple of places in code base where we declare a field / variable as implementation type (i.e. HashMap, HashSet) instead of interface type (i.e. Map, Set), while in other places we do the opposite by declaring as interface type. A quick check indicates that most if not all of these places could be updated so we have a consistent style over the code base (prefer using interface type), which is also a good coding style to stick per best practice.

Checked and fixed Set, Map and List interface usages.

Author: Tamas Penzes <tamaas@cloudera.com>

Reviewers: Abe Fine <afine@apache.org>, Michael Han <hanm@apache.org>

Closes apache#354 from tamaashu/ZOOKEEPER-2630

(cherry picked from commit 1165794)
lvfangmin pushed a commit to lvfangmin/zookeeper that referenced this pull request Jun 17, 2018
Use interface type instead of implementation type when appropriate.

There are a couple of places in code base where we declare a field / variable as implementation type (i.e. HashMap, HashSet) instead of interface type (i.e. Map, Set), while in other places we do the opposite by declaring as interface type. A quick check indicates that most if not all of these places could be updated so we have a consistent style over the code base (prefer using interface type), which is also a good coding style to stick per best practice.

Checked and fixed Set, Map and List interface usages.

Author: Tamas Penzes <tamaas@cloudera.com>

Reviewers: Abe Fine <afine@apache.org>, Michael Han <hanm@apache.org>

Closes apache#354 from tamaashu/ZOOKEEPER-2630
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
Use interface type instead of implementation type when appropriate.

There are a couple of places in code base where we declare a field / variable as implementation type (i.e. HashMap, HashSet) instead of interface type (i.e. Map, Set), while in other places we do the opposite by declaring as interface type. A quick check indicates that most if not all of these places could be updated so we have a consistent style over the code base (prefer using interface type), which is also a good coding style to stick per best practice.

Checked and fixed Set, Map and List interface usages.

Author: Tamas Penzes <tamaas@cloudera.com>

Reviewers: Abe Fine <afine@apache.org>, Michael Han <hanm@apache.org>

Closes apache#354 from tamaashu/ZOOKEEPER-2630
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants