[
https://issues.apache.org/jira/browse/GEODE-2936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16049663#comment-16049663
]
ASF GitHub Bot commented on GEODE-2936:
---------------------------------------
Github user jaredjstewart commented on a diff in the pull request:
https://github.com/apache/geode/pull/580#discussion_r122066276
--- Diff:
geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java
---
@@ -228,4 +139,55 @@ void addEvaluatedSortCriteria(Object row,
ExecutionContext context)
// No op
}
+ private int compareHelperMethod(Object obj1, Object obj2) {
+ if (obj1 == null || obj2 == null) {
+ return compareIfOneOrMoreNull(obj1, obj2);
+ } else if (obj1 == QueryService.UNDEFINED || obj2 ==
QueryService.UNDEFINED) {
+ return compareIfOneOrMoreQueryServiceUndefined(obj1, obj2);
+ } else {
+ return compareTwoObjects(obj1, obj2);
+ }
+ }
+
+ private int compareIfOneOrMoreNull(Object obj1, Object obj2) {
+ if (obj1 == null) {
+ return obj2 == null ? 0 : -1;
+ } else {
+ return 1;
+ }
+ }
+
+ private int compareIfOneOrMoreQueryServiceUndefined(Object obj1, Object
obj2) {
+ if (obj1 == QueryService.UNDEFINED) {
+ return obj2 == QueryService.UNDEFINED ? 0 : -1;
+ } else {
+ return 1;
+ }
+ }
+
+ private int compareTwoObjects(Object obj1, Object obj2) {
+ if (obj1 instanceof Number && obj2 instanceof Number) {
+ return compareTwoNumbers(obj1, obj2);
+ } else {
+ return compareTwoStrings(obj1, obj2);
+ }
+ }
+
+ private int compareTwoNumbers(Object obj1, Object obj2) {
--- End diff --
I see it was not introduced by your changes, but the subtraction on 177 is
not a safe way to compare doubles as it can result in under/overflow. For
example, consider the result of `compareTwoNumbers( Double.MAX_VALUE, -1d)`.
Here's a safer way to do it:
```
private int compareTwoNumbers(Object obj1, Object obj2) {
Number number1 = (Number) obj1;
Number number2 = (Number) obj2 ;
return Double.compare(number1.doubleValue(), number2.doubleValue());
}
```
> 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)
