Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#12890 closed enhancement (fixed)

Use Java 8 Predicates

Reported by: michael2402 Owned by: Don-vip
Priority: normal Milestone: 16.07
Component: Core Version:
Keywords: gsoc-core predicates java8 Cc: Don-vip, bastiK, stoecker

Description

To make full use of Java 8, we often need predicates.

I suggest making org.openstreetmap.josm.tools.Predicate<T> extend java.util.function.Predicate<T>

We then delegate test to evaluate.

All utility methods return josm predicates but take normal java predicates as arguments. That way, we can switch to java predicates while maintaining compatibility.

Attachments (0)

Change History (18)

comment:1 Changed 3 years ago by Don-vip

Milestone: 16.06

comment:2 Changed 3 years ago by bastiK

What about fields like OsmPrimitive.modifiedPredicate?

comment:3 Changed 3 years ago by michael2402

We can use josm predicates for the constants, so they can be used both as java and as josm predicate.

I don't think that we should keep modifiedPredicate and others with Java8. I don't see any advantage of them. Compare it yourself:

list.filter(OsmPrimitive.modifiedPredicate);
list.filter(n -> n.isModified());

Speed would be approx. the same in both cases, the second one has lower memory footprint (since GC can collect the predicate fast)

Read/Write fields should have getters/setters any way.

We can add a wrapper to convert a Predicate to a josm Predicate, that way we can use:

...josm...Predicate getXXXPredicate()
setXXXPredicate(...java...Predicate)
Last edited 3 years ago by michael2402 (previous) (diff)

comment:4 in reply to:  3 ; Changed 3 years ago by bastiK

Replying to michael2402:

We can use josm predicates for the constants, so they can be used both as java and as josm predicate.

I don't think that we should keep modifiedPredicate and others with Java8. I don't see any advantage of them. Compare it yourself:

list.filter(OsmPrimitive.modifiedPredicate);
list.filter(n -> n.isModified());

Speed would be approx. the same in both cases, the second one has lower memory footprint (since GC can collect the predicate fast)

Neat, but others (e.g. multipolygonPredicate) we probably want to keep. The point of making org.openstreetmap.josm.tools.Predicate extend java.util.function.Predicate is to have a transition period, where plugins can be fixed. When plugins are ready, we get rid of org.openstreetmap.josm.tools.Predicate completely.

At that point you have to change the type of multipolygonPredicate, which breaks any plugins that still use the field.

  1. If plugins are broken at some point, we might as well do the complete switch at once and be done with it.
  2. A smooth transition as you planned would be better. I guess, one solution is to duplicate all constants. Plugins are fixed by switching to the new set. The old set of constants is then unused and can be removed. Disadvantage: requires name change or move to new class.

comment:5 in reply to:  4 Changed 3 years ago by michael2402

Replying to bastiK:

Neat, but others (e.g. multipolygonPredicate) we probably want to keep

By adding a new isMultipolygon method, multipolygonPredicate can be expressed in a nicer way:

p -> p.isMultipolygon()

We do not even need the class comparison and the cast to Relation, we can use normal inheritance there by simply adding the isMultipolygon method to all primitives and make it return false except for relations.

Replying to bastiK:

  1. A smooth transition as you planned would be better. I guess, one solution is to duplicate all constants. Plugins are fixed by switching to the new set. The old set of constants is then unused and can be removed. Disadvantage: requires name change or move to new class.

We can use this to fix the constant names ;-).

But we do not need to change any names or have any dupplicate values.

Old plugin code:

...josm...Predicate<T> pred = SOME_CONSTANT;
...josm...Predicate<T> pred2 = Predicates.not(SOME_CONSTANT);

This can simply be changed to:

...java...Predicate<T> pred = SOME_CONSTANT;
...java...Predicate<T> pred2 = Predicates.not(SOME_CONSTANT);

SOME_CONSTANT is a ...josm...Predicate<T> which extends ...java...Predicate<T>. Predicates.not has this signature: ...josm...Predicate<T> not(...java...Predicate<T>)

The plugin does not break when enabling Java predicates and we can use them in mixed mode for some time.

comment:6 Changed 3 years ago by bastiK

As far as I know, this still breaks plugins when the type of the field is changed. It doesn't matter what it is assigned to, just access is enough to raise NoSuchFieldError.

Anyway, one way or another, this is going to work. Changing to lambda expressions should remove most references to the constant fields.

Idea: Have 2 versions of Main.map.mapView.getNearestNode (and similar functions), one with java Predicate and one with josm Predicate as argument type. Deprecate the JOSM version -> Plugin developers will get a fixable deprecation warning.

comment:7 Changed 3 years ago by Don-vip

Milestone: 16.0616.07

comment:8 Changed 3 years ago by Don-vip

Owner: changed from team to Don-vip
Status: newassigned

comment:9 Changed 3 years ago by Don-vip

In 10657/josm:

see #11390, see #12890 - use Java 8 Predicates

comment:10 Changed 3 years ago by Don-vip

In 10658/josm:

see #11390, see #12890 - use Java 8 Predicates (forgot some)

comment:11 Changed 3 years ago by Don-vip

In 10689/josm:

see #11390, see #12890 - Use Java 8 Function

comment:12 Changed 3 years ago by Don-vip

Resolution: fixed
Status: assignedclosed

In 10691/josm:

see #11390, fix #12890 - finish transition to Java 8 predicates/functions

comment:13 Changed 3 years ago by simon04

In 10715/josm:

see #11390, see #12890 - Deprecate Predicates class

comment:14 Changed 3 years ago by simon04

In 10716/josm:

see #11390, see #12890 - Deprecate predicates in OsmPrimitive class

comment:15 Changed 3 years ago by simon04

In 10717/josm:

see #11390, see #12890 - Lambda can be replaced with method reference

comment:16 Changed 3 years ago by Don-vip

In 10744/josm:

see #12890 - remove calls to deprecated predicates in unit tests

comment:17 Changed 3 years ago by simon04

In 10774/josm:

see #11390, see #12890 - Drop Match.{existsMatch,forallMatch} in favour of stream API

comment:18 Changed 3 years ago by Don-vip

In 10834/josm:

see #12890 - remove Predicates class (not used anymore by known plugins)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.