[1/2] groovy git commit: relocate helper method

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[1/2] groovy git commit: relocate helper method

jwagenleitner-2
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_5_X 26102747f -> e5f461be0


relocate helper method

Helper method was used exclusively by one class and did not justify
itself to be a publicly accessible method.


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/e5f461be
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/e5f461be
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/e5f461be

Branch: refs/heads/GROOVY_2_5_X
Commit: e5f461be0e7dc453c713ba1a7aa7f8c64a0fef4b
Parents: ce72a79
Author: John Wagenleitner <[hidden email]>
Authored: Sun Jul 16 12:49:32 2017 -0700
Committer: John Wagenleitner <[hidden email]>
Committed: Sun Jul 16 13:20:19 2017 -0700

----------------------------------------------------------------------
 .../org/codehaus/groovy/ast/tools/GeneralUtils.java | 11 -----------
 .../groovy/transform/SortableASTTransformation.java | 16 +++++++++++++---
 2 files changed, 13 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/e5f461be/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
index 9280a30..5b49a12 100644
--- a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -229,17 +229,6 @@ public class GeneralUtils {
         return new BinaryExpression(lhv, CMP, rhv);
     }
 
-    /**
-     * Build a binary expression that compares two values
-     * @param lhv expression for the value to compare from
-     * @param rhv expression for the value value to compare to
-     * @param reversed whether to use natural ordering or reversed natural ordering
-     * @return the expression comparing two values
-     */
-    public static BinaryExpression cmpX(Expression lhv, Expression rhv, boolean reversed) {
-        return (reversed) ? cmpX(rhv, lhv) : cmpX(lhv, rhv);
-    }
-
     public static ConstantExpression constX(Object val) {
         return new ConstantExpression(val);
     }

http://git-wip-us.apache.org/repos/asf/groovy/blob/e5f461be/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java b/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
index ab884a1..643feaa 100644
--- a/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
@@ -29,6 +29,8 @@ import org.codehaus.groovy.ast.InnerClassNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.PropertyNode;
+import org.codehaus.groovy.ast.expr.BinaryExpression;
+import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.classgen.VariableScopeVisitor;
 import org.codehaus.groovy.runtime.AbstractComparator;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
@@ -124,14 +126,14 @@ public class SortableASTTransformation extends AbstractASTTransformation {
             // return this.hashCode() <=> other.hashCode()
             statements.add(declS(varX(THIS_HASH, ClassHelper.Integer_TYPE), callX(varX("this"), "hashCode")));
             statements.add(declS(varX(OTHER_HASH, ClassHelper.Integer_TYPE), callX(varX(OTHER), "hashCode")));
-            statements.add(returnS(cmpX(varX(THIS_HASH), varX(OTHER_HASH), reversed)));
+            statements.add(returnS(compareExpr(varX(THIS_HASH), varX(OTHER_HASH), reversed)));
         } else {
             // int value = 0;
             statements.add(declS(varX(VALUE, ClassHelper.int_TYPE), constX(0)));
             for (PropertyNode property : properties) {
                 String propName = property.getName();
                 // value = this.prop <=> other.prop;
-                statements.add(assignS(varX(VALUE), cmpX(propX(varX("this"), propName), propX(varX(OTHER), propName), reversed)));
+                statements.add(assignS(varX(VALUE), compareExpr(propX(varX("this"), propName), propX(varX(OTHER), propName), reversed)));
                 // if (value != 0) return value;
                 statements.add(ifS(neX(varX(VALUE), constX(0)), returnS(varX(VALUE))));
             }
@@ -154,7 +156,7 @@ public class SortableASTTransformation extends AbstractASTTransformation {
                 // if (arg0 == null && arg1 != null) return 1;
                 ifS(andX(equalsNullX(varX(ARG0)), notNullX(varX(ARG1))), returnS(constX(1))),
                 // return arg0.prop <=> arg1.prop;
-                returnS(cmpX(propX(varX(ARG0), propName), propX(varX(ARG1), propName), reversed))
+                returnS(compareExpr(propX(varX(ARG0), propName), propX(varX(ARG1), propName), reversed))
         );
     }
 
@@ -223,4 +225,12 @@ public class SortableASTTransformation extends AbstractASTTransformation {
                 pNode.getName() + "' must be Comparable", pNode);
     }
 
+    /**
+     * Helper method used to build a binary expression that compares two values
+     * with the option to handle reverse order.
+     */
+    private static BinaryExpression compareExpr(Expression lhv, Expression rhv, boolean reversed) {
+        return (reversed) ? cmpX(rhv, lhv) : cmpX(lhv, rhv);
+    }
+
 }

Reply | Threaded
Open this post in threaded view
|

[2/2] groovy git commit: GROOVY-8218 @Sortable allows reversed natural ordering (closes #558)

jwagenleitner-2
GROOVY-8218 @Sortable allows reversed natural ordering (closes #558)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/ce72a79a
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/ce72a79a
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/ce72a79a

Branch: refs/heads/GROOVY_2_5_X
Commit: ce72a79adbc6da6ea94c1ad80f7189c52698cd90
Parents: 2610274
Author: Paul-Julien Vauthier <[hidden email]>
Authored: Sun Jun 4 14:31:48 2017 +0800
Committer: John Wagenleitner <[hidden email]>
Committed: Sun Jul 16 13:20:19 2017 -0700

----------------------------------------------------------------------
 src/main/groovy/transform/Sortable.java         | 31 ++++++++++++++++++++
 .../codehaus/groovy/ast/tools/GeneralUtils.java | 17 +++++++++++
 .../transform/SortableASTTransformation.java    | 19 ++++++------
 .../transform/SortableTransformTest.groovy      | 23 +++++++++++++++
 4 files changed, 81 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/ce72a79a/src/main/groovy/transform/Sortable.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/transform/Sortable.java b/src/main/groovy/transform/Sortable.java
index 45fe99f..b27013d 100644
--- a/src/main/groovy/transform/Sortable.java
+++ b/src/main/groovy/transform/Sortable.java
@@ -45,6 +45,7 @@ import org.codehaus.groovy.transform.GroovyASTTransformationClass;
  *     priority will be according to the order given in the includes list)</li>
  *     <li>have three {@code Comparator} methods named {@code comparatorByFirst},
  *     {@code comparatorByLast} and {@code comparatorByBorn}</li>
+ *     <li>sort by natural order by default, reversed natural order can be specified</li>
  * </ul>
  * The properties within the class must themselves be {@code Comparable} or {@code @Sortable}.
  * <p>More examples:</p>
@@ -123,6 +124,30 @@ import org.codehaus.groovy.transform.GroovyASTTransformationClass;
  * assert sortedByMaxAttendees[2].beginDate &lt; sortedByMaxAttendees[1].beginDate
  *
  * assert Course.declaredMethods.name.findAll { it.startsWith('comparatorBy') }.toSorted() == ['comparatorByMaxAttendees', 'comparatorByTitle']
+ *
+ * //--------------------------------------------------------------------------
+ * // Sorting by max attendees using reversed order
+ * import groovy.transform.Sortable
+ * import groovy.transform.ToString
+ *
+ * &#64;Sortable(includes = ['points'], reversed = true)
+ * &#64;ToString
+ * class LeaderBoardEntry {
+ *     String team
+ *     int points
+ * }
+ *
+ *
+ * final LeaderBoardEntry teamA = new LeaderBoardEntry(team: "Team A", points: 30)
+ * final LeaderBoardEntry teamB = new LeaderBoardEntry(team: "Team B", points: 80)
+ * final LeaderBoardEntry teamC = new LeaderBoardEntry(team: "Team C", points: 50)
+ *
+ * final List&lt;LeaderBoardEntry&gt; leaderBoard = [teamA, teamB, teamC].toSorted()
+ *
+ * assert leaderBoard.first().team == 'Team B'
+ * assert leaderBoard.last().team == 'Team A'
+ * assert leaderBoard.points == [80, 50, 30]
+ *
  * </pre>
  *
  * @author Andres Almiray
@@ -145,4 +170,10 @@ public @interface Sortable {
      * Must not be used if 'includes' is used.
      */
     String[] excludes() default {};
+
+    /**
+     * Set to true so that comparator uses reversed natural order.
+     * @since 2.5.0
+     */
+    boolean reversed() default false;
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/ce72a79a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
index c00a62d..9280a30 100644
--- a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -219,10 +219,27 @@ public class GeneralUtils {
         return result;
     }
 
+    /**
+     * Build a binary expression that compares two values
+     * @param lhv expression for the value to compare from
+     * @param rhv expression for the value value to compare to
+     * @return the expression comparing two values
+     */
     public static BinaryExpression cmpX(Expression lhv, Expression rhv) {
         return new BinaryExpression(lhv, CMP, rhv);
     }
 
+    /**
+     * Build a binary expression that compares two values
+     * @param lhv expression for the value to compare from
+     * @param rhv expression for the value value to compare to
+     * @param reversed whether to use natural ordering or reversed natural ordering
+     * @return the expression comparing two values
+     */
+    public static BinaryExpression cmpX(Expression lhv, Expression rhv, boolean reversed) {
+        return (reversed) ? cmpX(rhv, lhv) : cmpX(lhv, rhv);
+    }
+
     public static ConstantExpression constX(Object val) {
         return new ConstantExpression(val);
     }

http://git-wip-us.apache.org/repos/asf/groovy/blob/ce72a79a/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java b/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
index c48f2a1..ab884a1 100644
--- a/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/SortableASTTransformation.java
@@ -82,6 +82,7 @@ public class SortableASTTransformation extends AbstractASTTransformation {
     private void createSortable(AnnotationNode annotation, ClassNode classNode) {
         List<String> includes = getMemberStringList(annotation, "includes");
         List<String> excludes = getMemberStringList(annotation, "excludes");
+        boolean reversed = memberHasValue(annotation, "reversed", true);
         if (!checkIncludeExcludeUndefinedAware(annotation, excludes, includes, MY_TYPE_NAME)) return;
         if (!checkPropertyList(classNode, includes, "includes", annotation, MY_TYPE_NAME, false)) return;
         if (!checkPropertyList(classNode, excludes, "excludes", annotation, MY_TYPE_NAME, false)) return;
@@ -97,11 +98,11 @@ public class SortableASTTransformation extends AbstractASTTransformation {
                 ClassHelper.int_TYPE,
                 params(param(newClass(classNode), OTHER)),
                 ClassNode.EMPTY_ARRAY,
-                createCompareToMethodBody(properties)
+                createCompareToMethodBody(properties, reversed)
         ));
 
         for (PropertyNode property : properties) {
-            createComparatorFor(classNode, property);
+            createComparatorFor(classNode, property, reversed);
         }
         new VariableScopeVisitor(sourceUnit, true).visitClass(classNode);
     }
@@ -112,7 +113,7 @@ public class SortableASTTransformation extends AbstractASTTransformation {
         }
     }
 
-    private static Statement createCompareToMethodBody(List<PropertyNode> properties) {
+    private static Statement createCompareToMethodBody(List<PropertyNode> properties, boolean reversed) {
         List<Statement> statements = new ArrayList<Statement>();
 
         // if (this.is(other)) return 0;
@@ -123,14 +124,14 @@ public class SortableASTTransformation extends AbstractASTTransformation {
             // return this.hashCode() <=> other.hashCode()
             statements.add(declS(varX(THIS_HASH, ClassHelper.Integer_TYPE), callX(varX("this"), "hashCode")));
             statements.add(declS(varX(OTHER_HASH, ClassHelper.Integer_TYPE), callX(varX(OTHER), "hashCode")));
-            statements.add(returnS(cmpX(varX(THIS_HASH), varX(OTHER_HASH))));
+            statements.add(returnS(cmpX(varX(THIS_HASH), varX(OTHER_HASH), reversed)));
         } else {
             // int value = 0;
             statements.add(declS(varX(VALUE, ClassHelper.int_TYPE), constX(0)));
             for (PropertyNode property : properties) {
                 String propName = property.getName();
                 // value = this.prop <=> other.prop;
-                statements.add(assignS(varX(VALUE), cmpX(propX(varX("this"), propName), propX(varX(OTHER), propName))));
+                statements.add(assignS(varX(VALUE), cmpX(propX(varX("this"), propName), propX(varX(OTHER), propName), reversed)));
                 // if (value != 0) return value;
                 statements.add(ifS(neX(varX(VALUE), constX(0)), returnS(varX(VALUE))));
             }
@@ -143,7 +144,7 @@ public class SortableASTTransformation extends AbstractASTTransformation {
         return body;
     }
 
-    private static Statement createCompareMethodBody(PropertyNode property) {
+    private static Statement createCompareMethodBody(PropertyNode property, boolean reversed) {
         String propName = property.getName();
         return block(
                 // if (arg0 == arg1) return 0;
@@ -153,11 +154,11 @@ public class SortableASTTransformation extends AbstractASTTransformation {
                 // if (arg0 == null && arg1 != null) return 1;
                 ifS(andX(equalsNullX(varX(ARG0)), notNullX(varX(ARG1))), returnS(constX(1))),
                 // return arg0.prop <=> arg1.prop;
-                returnS(cmpX(propX(varX(ARG0), propName), propX(varX(ARG1), propName)))
+                returnS(cmpX(propX(varX(ARG0), propName), propX(varX(ARG1), propName), reversed))
         );
     }
 
-    private static void createComparatorFor(ClassNode classNode, PropertyNode property) {
+    private static void createComparatorFor(ClassNode classNode, PropertyNode property, boolean reversed) {
         String propName = property.getName();
         String className = classNode.getName() + "$" + StringGroovyMethods.capitalize(propName) + "Comparator";
         ClassNode superClass = makeClassSafeWithGenerics(AbstractComparator.class, classNode);
@@ -170,7 +171,7 @@ public class SortableASTTransformation extends AbstractASTTransformation {
                 ClassHelper.int_TYPE,
                 params(param(newClass(classNode), ARG0), param(newClass(classNode), ARG1)),
                 ClassNode.EMPTY_ARRAY,
-                createCompareMethodBody(property)
+                createCompareMethodBody(property, reversed)
         ));
 
         String fieldName = "this$" + StringGroovyMethods.capitalize(propName) + "Comparator";

http://git-wip-us.apache.org/repos/asf/groovy/blob/ce72a79a/src/test/org/codehaus/groovy/transform/SortableTransformTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/transform/SortableTransformTest.groovy b/src/test/org/codehaus/groovy/transform/SortableTransformTest.groovy
index 6a0540e..9a80f57 100644
--- a/src/test/org/codehaus/groovy/transform/SortableTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/SortableTransformTest.groovy
@@ -166,4 +166,27 @@ class SortableTransformTest extends CompilableTestSupport {
         '''
         assert message.contains("@Sortable cannot be applied to interface Foo")
     }
+
+    void testReverseSorting() {
+        assertScript '''
+            import groovy.transform.*
+
+            @CompileStatic
+            @Canonical
+            @Sortable(includes = ['age'], reversed = true)
+            class Person {
+                String name
+                int age
+            }
+
+            def persons = [
+                new Person('PJ', 25),
+                new Person('Guillaume', 40)
+            ]
+
+            assert persons*.age == [25, 40]
+            assert persons.sort()*.age == [40, 25]
+            assert persons.sort(false, Person.comparatorByAge())*.age == [40, 25]
+        '''
+    }
 }