You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/03/03 19:36:42 UTC
[GitHub] [accumulo] jmark99 opened a new pull request #1541: Replace use of
equals() with Object.equals()
jmark99 opened a new pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541
Utilize newer Java language features to simplfy code by using Object.equals() rather than equals().
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] jmark99 commented on a change in pull request #1541:
Replace use of equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541#discussion_r387702672
##########
File path: core/src/main/java/org/apache/accumulo/core/data/Condition.java
##########
@@ -308,41 +309,20 @@ public Condition setIterators(IteratorSetting... iterators) {
@Override
public boolean equals(Object o) {
- if (o == this) {
+ if (this == o)
return true;
- }
- if (o == null || !(o instanceof Condition)) {
- return false;
- }
- Condition c = (Condition) o;
- if (!(c.cf.equals(cf))) {
- return false;
- }
- if (!(c.cq.equals(cq))) {
- return false;
- }
- if (!(c.cv.equals(cv))) {
+ if (o == null || getClass() != o.getClass())
Review comment:
@ctubbsii, I reverted the aforementioned check back to its original form. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1541:
Replace use of equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541#discussion_r387694367
##########
File path: core/src/main/java/org/apache/accumulo/core/data/Condition.java
##########
@@ -308,41 +309,20 @@ public Condition setIterators(IteratorSetting... iterators) {
@Override
public boolean equals(Object o) {
- if (o == this) {
+ if (this == o)
return true;
- }
- if (o == null || !(o instanceof Condition)) {
- return false;
- }
- Condition c = (Condition) o;
- if (!(c.cf.equals(cf))) {
- return false;
- }
- if (!(c.cq.equals(cq))) {
- return false;
- }
- if (!(c.cv.equals(cv))) {
+ if (o == null || getClass() != o.getClass())
Review comment:
This was changed from `instanceof` to the use of `getClass()`, which is not the same. The latter is more narrow. I think it should be changed back to `instanceof`, although the check for null would be redundant, since `instanceof` is always false if it's null.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] EdColeman commented on a change in pull request #1541:
Replace use of equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541#discussion_r387294444
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/DiskUsage.java
##########
@@ -47,9 +48,9 @@ public boolean equals(Object o) {
DiskUsage diskUsage = (DiskUsage) o;
- if (tables != null ? !tables.equals(diskUsage.tables) : diskUsage.tables != null)
+ if (!Objects.equals(tables, diskUsage.tables))
return false;
- return usage != null ? usage.equals(diskUsage.usage) : diskUsage.usage == null;
+ return Objects.equals(usage, diskUsage.usage);
Review comment:
I think the same thing could apply to the other 2 classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1541:
Replace use of equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541#discussion_r387842239
##########
File path: core/src/main/java/org/apache/accumulo/core/data/Condition.java
##########
@@ -308,41 +309,20 @@ public Condition setIterators(IteratorSetting... iterators) {
@Override
public boolean equals(Object o) {
- if (o == this) {
+ if (this == o)
return true;
- }
- if (o == null || !(o instanceof Condition)) {
- return false;
- }
- Condition c = (Condition) o;
- if (!(c.cf.equals(cf))) {
- return false;
- }
- if (!(c.cq.equals(cq))) {
- return false;
- }
- if (!(c.cv.equals(cv))) {
+ if (o == null || !(o instanceof Condition))
Review comment:
This could be simplified to:
```suggestion
if (!(o instanceof Condition))
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] EdColeman commented on a change in pull request #1541:
Replace use of equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541#discussion_r387270591
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/DiskUsage.java
##########
@@ -47,9 +48,9 @@ public boolean equals(Object o) {
DiskUsage diskUsage = (DiskUsage) o;
- if (tables != null ? !tables.equals(diskUsage.tables) : diskUsage.tables != null)
+ if (!Objects.equals(tables, diskUsage.tables))
return false;
- return usage != null ? usage.equals(diskUsage.usage) : diskUsage.usage == null;
+ return Objects.equals(usage, diskUsage.usage);
Review comment:
This could be reduced further using AND, something like:
`return Objects.equals(tables, diskUsage.tables) && Objects.equals(usage, diskUsage.usage);`
Also, any thought to replacing hashCode at the same time to use Objects.hash(tables, usage); - that might help with the constraint that equals and hashCode are consistent.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] jmark99 merged pull request #1541: Replace use of
equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
jmark99 merged pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] jmark99 commented on a change in pull request #1541:
Replace use of equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541#discussion_r387293749
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/DiskUsage.java
##########
@@ -47,9 +48,9 @@ public boolean equals(Object o) {
DiskUsage diskUsage = (DiskUsage) o;
- if (tables != null ? !tables.equals(diskUsage.tables) : diskUsage.tables != null)
+ if (!Objects.equals(tables, diskUsage.tables))
return false;
- return usage != null ? usage.equals(diskUsage.usage) : diskUsage.usage == null;
+ return Objects.equals(usage, diskUsage.usage);
Review comment:
Thanks @EdColeman . Updated to add further simplification and updated hashCode.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] jmark99 commented on a change in pull request #1541:
Replace use of equals() with Object.equals()
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #1541: Replace use of equals() with Object.equals()
URL: https://github.com/apache/accumulo/pull/1541#discussion_r387688215
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/DiskUsage.java
##########
@@ -47,9 +48,9 @@ public boolean equals(Object o) {
DiskUsage diskUsage = (DiskUsage) o;
- if (tables != null ? !tables.equals(diskUsage.tables) : diskUsage.tables != null)
+ if (!Objects.equals(tables, diskUsage.tables))
return false;
- return usage != null ? usage.equals(diskUsage.usage) : diskUsage.usage == null;
+ return Objects.equals(usage, diskUsage.usage);
Review comment:
@EdColeman pushed up changes to the other two classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services