[ 
https://issues.apache.org/jira/browse/GEODE-2936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16102268#comment-16102268
 ] 

ASF GitHub Bot commented on GEODE-2936:
---------------------------------------

Github user kirklund commented on a diff in the pull request:

    https://github.com/apache/geode/pull/580#discussion_r129693040
  
    --- Diff: 
geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
 ---
    @@ -173,36 +157,54 @@ public void testUnsupportedOrderByForPR() throws 
Exception {
     
       @Test
       public void testSupportedOrderByForRR() throws Exception {
    -
         String unsupportedQueries[] =
    -        {"select distinct p.status from /portfolio1 p order by p.status, 
p.ID",
    -
    -        };
    +        {"select distinct p.status from /portfolio1 p order by p.status, 
p.ID"};
         Object r[][] = new Object[unsupportedQueries.length][2];
    -    QueryService qs;
    -    qs = CacheUtils.getQueryService();
         Position.resetCounter();
    -    // Create Regions
     
    +    // Create Regions
         Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
     
         for (int i = 0; i < 50; i++) {
           r1.put(new Portfolio(i), new Portfolio(i));
         }
     
         for (int i = 0; i < unsupportedQueries.length; i++) {
    -      Query q = null;
    -
    +      Query q;
           CacheUtils.getLogger().info("Executing query: " + 
unsupportedQueries[i]);
           q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
           try {
             r[i][0] = q.execute();
    -
           } catch (QueryInvalidException qe) {
             qe.printStackTrace();
             fail(qe.toString());
           }
         }
       }
     
    +  // The following tests cover edge cases in OrderByComparator.
    +  @Test
    +  public void testCompareTwoNulls() throws Exception {
    +    assert (createComparator().compare(null, null) == 0);
    --- End diff --
    
    "assert" should be used in product code instead of tests. These are the 
optional assertions that are enabled when running a Java product with -ea 
(enable assertions).
    
    You should use assertThat instead:
    ```java
    assertThat(createComparator().compare(null, null).isEqualTo(0);
    ```


> Refactor OrderByComparator's compare method to reduce redundant code
> --------------------------------------------------------------------
>
>                 Key: GEODE-2936
>                 URL: https://issues.apache.org/jira/browse/GEODE-2936
>             Project: Geode
>          Issue Type: Bug
>          Components: querying
>            Reporter: nabarun
>            Assignee: Emily Yeh
>
> Issue:
> OrderByComparator's compare method has a lot of redundant code.
> Solution:
> These code sections can be modified to have one method call



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to