Language level updated to java 8 #102

balazsjonas · 2020-04-25T22:09:01Z

Language level updated to 8
using java.util.Optional, java.util.Objects instead of guava-mini

codecov-io · 2020-04-25T22:10:36Z

Codecov Report

Merging #102 into master will increase coverage by 6.25%.
The diff coverage is 75.43%.

@@             Coverage Diff              @@
##             master     #102      +/-   ##
============================================
+ Coverage     75.88%   82.14%   +6.25%     
+ Complexity      824      808      -16     
============================================
  Files            72       71       -1     
  Lines          2322     2055     -267     
  Branches        312      268      -44     
============================================
- Hits           1762     1688      -74     
+ Misses          472      283     -189     
+ Partials         88       84       -4

Impacted Files	Coverage Δ	Complexity Δ
.../com/github/davidmoten/rtree/SerializerHelper.java	`100.00% <ø> (ø)`	`1.00 <0.00> (-1.00)`
...thub/davidmoten/rtree/internal/NodeAndEntries.java	`100.00% <ø> (ø)`	`4.00 <0.00> (ø)`
...m/github/davidmoten/rtree/kryo/SerializerKryo.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...avidmoten/rtree/geometry/internal/PointDouble.java	`54.76% <31.57%> (-19.16%)`	`21.00 <3.00> (+3.00)`	⬇️
...vidmoten/rtree/geometry/internal/CircleDouble.java	`60.00% <33.33%> (ø)`	`11.00 <1.00> (ø)`
...davidmoten/rtree/geometry/internal/LineDouble.java	`38.77% <33.33%> (ø)`	`12.00 <1.00> (ø)`
.../davidmoten/rtree/geometry/internal/LineFloat.java	`91.83% <33.33%> (ø)`	`22.00 <1.00> (ø)`
...moten/rtree/geometry/internal/RectangleDouble.java	`84.09% <33.33%> (ø)`	`25.00 <1.00> (ø)`
...om/github/davidmoten/rtree/internal/Functions.java	`33.33% <33.33%> (+8.33%)`	`1.00 <1.00> (ø)`
...com/github/davidmoten/rtree/SplitterQuadratic.java	`95.91% <77.77%> (ø)`	`19.00 <9.00> (ø)`
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 569cf17...dfdb872. Read the comment docs.

davidmoten · 2020-04-26T01:37:13Z

Yeah I think Java 8 support is fine. We were staying with Java 6 support for Android use but I think Java 8 is usable on Android now.

davidmoten · 2020-04-26T01:38:05Z

src/main/java/com/github/davidmoten/rtree/RTree.java

 import rx.Observable;
 import rx.functions.Func1;
 import rx.functions.Func2;

+import java.util.*;


no wildcard imports please

Sorry, I used default setting of my ide. Do you have some style guide?
For a quick solution I increased the "class count to use import with '*'" to 10 in intelliJ

no style guide, you can infer stuff like that from the code in front of you though.

davidmoten · 2020-04-26T01:38:40Z

src/main/java/com/github/davidmoten/rtree/RTree.java

-import com.github.davidmoten.rtree.geometry.Line;
-import com.github.davidmoten.rtree.geometry.Point;
-import com.github.davidmoten.rtree.geometry.Rectangle;
+import com.github.davidmoten.rtree.geometry.*;


no wildcard imports please

davidmoten

Thank for the contribution. Would be helpful if you stick to your stated aim rather than do lots of random whitespace changes (javadoc changes were a time wasting distraction to review). You could have left a lot of the code unchanged by simply changing a static import of guavamini Optional.of to java.util.Optional.of (again about not making me review stuff that I didn't need to). Thanks for adding the curly braces, I'm happy with those and nice to git rid of those non-lambda anonymous classes.

davidmoten · 2020-04-26T01:41:38Z

src/main/java/com/github/davidmoten/rtree/RTree.java

-     * 
-     * @param entry
-     *            item to add to the R-tree.
+     *


please don't reformat javadoc, it's got nothing to do with your PR

OK. In case of the static methods named "of" personally I prefer fully name, but in this case only Optional.of is used.
I will also check javadoc against master.

src/main/java/com/github/davidmoten/rtree/RTree.java

davidmoten · 2020-04-26T01:44:01Z

src/main/java/com/github/davidmoten/rtree/Visualizer.java

-import java.util.Collections;
-import java.util.Comparator;
-import java.util.List;
+import java.util.*;


no wildcard imports please

davidmoten · 2020-04-26T01:47:23Z

src/main/java/com/github/davidmoten/rtree/internal/Functions.java

@@ -9,30 +9,15 @@ private Functions() {
    }

    public static <T> Func1<T, T> identity() {


Now that you've drawn my attention to them, these methods should return singletons. I prefer holder pattern if you can be bothered to change them.

I agree, but for generic I cannot create static generic field like private static final Func1<T, T> IDENTITY = t -> t
Did you think something like this?

Please check solution of jdk: java.util.function.Function.identity()

Would be interesting to benchmark. I would expect that this would involve less allocation cost:

// actually use a holder pattern here private static final Func1<Object, Object> IDENTITY = t -> t; @SuppressWarnings("unchecked") public static final <T> Func1<T,T> identity() { return (Func1<T, T>) IDENTITY; }

At runtime no mucking around with generic types occurs so the cast would be to its already declared type which should be a no-op. Of course benchmarks are the real decider here.

BTW we don't need a benchmark of this fact added to this project. You can add it temporarily for your own tests but don't commit. (if you are going to benchmark of course).

davidmoten · 2020-04-26T01:48:22Z

src/main/java/com/github/davidmoten/rtree/internal/util/BoundedPriorityQueue.java

-import java.util.Collections;
-import java.util.Comparator;
-import java.util.List;
+import java.util.*;


No wildcard imports

davidmoten · 2020-04-26T01:50:26Z

src/test/java/com/github/davidmoten/rtree/RTreeTest.java

-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;


No wildcard imports

balazsjonas · 2020-05-01T13:10:48Z

src/main/java/com/github/davidmoten/rtree/RTree.java

-            return absent();
-        else
-            return of(root.get().geometry().mbr());
+        return root.map(r -> r.geometry().mbr());


As I remember you had a comment about this lambda which is disappeared somehow. Actually it can be constant if you worry about instantiating the lambda expression, but I think Optional.map is designed for this.

davidmoten · 2022-02-22T10:07:44Z

I have to apologize for letting this slip, you did some useful work. I'm upgrading dependencies and plugins right now. A rebased PR would be nice in a couple of days if you are willing, ta.

# Conflicts: # pom.xml

balazsjonas · 2022-02-22T13:35:28Z

I have to apologize for letting this slip, you did some useful work. I'm upgrading dependencies and plugins right now. A rebased PR would be nice in a couple of days if you are willing, ta.

No problem. I've just rebased the branch and checked that it still compiles. Tomorrow I will have more time to check the details and the comments. I don't remember too much.

davidmoten

I can see some left over changes not done yet like unrolling imports and minor naming. I'll approve and merge and fix myself, thanks!

davidmoten · 2022-02-22T22:40:03Z

src/main/java/com/github/davidmoten/rtree/RTree.java

@@ -56,10 +56,14 @@
     * Current size in Entries of the RTree.
     */
    private final int size;
+    private final Func2<Optional<Rectangle>, Entry<T, S>, Optional<Rectangle>> rectangleAccumulator =


should be static and called RECTANGLE_ACCUMULATOR

davidmoten · 2022-02-23T01:13:50Z

resolved in rtree 0.9 on Maven Central now

davidmoten reviewed Apr 26, 2020

View reviewed changes

davidmoten requested changes Apr 26, 2020

View reviewed changes

davidmoten changed the title ~~Language leve updated to java 8~~ Language level updated to java 8 Apr 26, 2020

balazsjonas force-pushed the java8 branch from 9b234ec to dfdb872 Compare May 1, 2020 11:43

balazsjonas commented May 1, 2020

View reviewed changes

updated to java 8

9230ebe

# Conflicts: # pom.xml

balazsjonas force-pushed the java8 branch from dfdb872 to 9230ebe Compare February 22, 2022 13:25

davidmoten approved these changes Feb 22, 2022

View reviewed changes

davidmoten merged commit af95682 into davidmoten:master Feb 22, 2022

balazsjonas deleted the java8 branch February 23, 2022 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Language level updated to java 8 #102

Language level updated to java 8 #102

balazsjonas commented Apr 25, 2020

codecov-io commented Apr 25, 2020 •

edited

Loading

davidmoten commented Apr 26, 2020

davidmoten Apr 26, 2020

balazsjonas Apr 26, 2020

davidmoten Apr 28, 2020

davidmoten Apr 26, 2020

davidmoten left a comment

davidmoten Apr 26, 2020

balazsjonas Apr 26, 2020

davidmoten Apr 26, 2020

davidmoten Apr 26, 2020

balazsjonas Apr 26, 2020

davidmoten Apr 28, 2020

davidmoten Apr 28, 2020

davidmoten Apr 26, 2020

davidmoten Apr 26, 2020

balazsjonas May 1, 2020

davidmoten commented Feb 22, 2022

balazsjonas commented Feb 22, 2022

davidmoten left a comment

davidmoten Feb 22, 2022

davidmoten commented Feb 23, 2022

		@@ -9,30 +9,15 @@ private Functions() {
		}

		public static <T> Func1<T, T> identity() {

Language level updated to java 8 #102

Language level updated to java 8 #102

Conversation

balazsjonas commented Apr 25, 2020

codecov-io commented Apr 25, 2020 • edited Loading

Codecov Report

davidmoten commented Apr 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidmoten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidmoten commented Feb 22, 2022

balazsjonas commented Feb 22, 2022

davidmoten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidmoten commented Feb 23, 2022

codecov-io commented Apr 25, 2020 •

edited

Loading