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/04/20 17:53:07 UTC
[GitHub] [accumulo] jmark99 opened a new pull request #1590: Use isEmpty() rather than size()/length() checks.
jmark99 opened a new pull request #1590:
URL: https://github.com/apache/accumulo/pull/1590
Per SonarQube recommendations:
Using Collection.size() to test for emptiness works, but using Collection.isEmpty() makes the code more readable and can be more performant. The time complexity of any isEmpty() method implementation should be O(1) whereas some implementations of size() can be O(n).
----------------------------------------------------------------
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
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1590: Use isEmpty() rather than size()/length() checks.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1590:
URL: https://github.com/apache/accumulo/pull/1590#discussion_r411626670
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/KerberosIT.java
##########
@@ -565,8 +565,8 @@ public void testRestartedMasterReusesSecretKey() throws Exception {
client.securityOperations().getDelegationToken(new DelegationTokenConfig());
assertTrue("Could not get tables with delegation token",
- mac.createAccumuloClient(rootUser.getPrincipal(), token).tableOperations().list()
- .size() > 0);
+ !mac.createAccumuloClient(rootUser.getPrincipal(), token).tableOperations().list()
+ .isEmpty());
Review comment:
This can be done in a future change.
----------------------------------------------------------------
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
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1590: Use isEmpty() rather than size()/length() checks.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1590:
URL: https://github.com/apache/accumulo/pull/1590#discussion_r411612072
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/KerberosIT.java
##########
@@ -565,8 +565,8 @@ public void testRestartedMasterReusesSecretKey() throws Exception {
client.securityOperations().getDelegationToken(new DelegationTokenConfig());
assertTrue("Could not get tables with delegation token",
- mac.createAccumuloClient(rootUser.getPrincipal(), token).tableOperations().list()
- .size() > 0);
+ !mac.createAccumuloClient(rootUser.getPrincipal(), token).tableOperations().list()
+ .isEmpty());
Review comment:
This is better, but a lot of these assert statements could be reversed so that instead of:
```java
assertTrue(! isEmpty() );
```
you get:
```java
assertFalse( isEmpty() );
```
----------------------------------------------------------------
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
[GitHub] [accumulo] jmark99 commented on a change in pull request #1590: Use isEmpty() rather than size()/length() checks.
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #1590:
URL: https://github.com/apache/accumulo/pull/1590#discussion_r411639834
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/KerberosIT.java
##########
@@ -565,8 +565,8 @@ public void testRestartedMasterReusesSecretKey() throws Exception {
client.securityOperations().getDelegationToken(new DelegationTokenConfig());
assertTrue("Could not get tables with delegation token",
- mac.createAccumuloClient(rootUser.getPrincipal(), token).tableOperations().list()
- .size() > 0);
+ !mac.createAccumuloClient(rootUser.getPrincipal(), token).tableOperations().list()
+ .isEmpty());
Review comment:
True! Will keep in mind for future update.
----------------------------------------------------------------
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