Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert stateful predicate distinctByKey into a list method #5462

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.apache.lucene.search.FuzzyQuery;
import org.apache.lucene.search.PrefixQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.TopScoreDocCollector;
import org.apache.lucene.search.suggest.document.Completion90PostingsFormat;
import org.apache.lucene.search.suggest.document.CompletionAnalyzer;
import org.apache.lucene.search.suggest.document.ContextQuery;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package org.opentripplanner.ext.geocoder;

import com.google.common.collect.Iterables;
import java.util.Collection;
import java.util.Optional;
import java.util.stream.Stream;
import org.opentripplanner.framework.collection.ListUtils;
import org.opentripplanner.framework.geometry.WgsCoordinate;
import org.opentripplanner.framework.i18n.I18NString;
import org.opentripplanner.framework.lang.PredicateUtils;
import org.opentripplanner.transit.model.site.StopLocation;
import org.opentripplanner.transit.model.site.StopLocationsGroup;
import org.opentripplanner.transit.service.TransitService;
Expand All @@ -29,7 +29,7 @@ class StopClusterMapper {
* - of "identical" stops which are very close to each other and have an identical name, only one
* is chosen (at random)
*/
Stream<StopCluster> generateStopClusters(
Iterable<StopCluster> generateStopClusters(
Collection<StopLocation> stopLocations,
Collection<StopLocationsGroup> stopLocationsGroups
) {
Expand All @@ -39,16 +39,20 @@ Stream<StopCluster> generateStopClusters(
.filter(sl -> sl.getParentStation() == null)
// stops without a name (for example flex areas) are useless for searching, so we remove them, too
.filter(sl -> sl.getName() != null)
// if they are very close to each other and have the same name, only one is chosen (at random)
.filter(
PredicateUtils.distinctByKey(sl ->
new DeduplicationKey(sl.getName(), sl.getCoordinate().roundToApproximate100m())
)
.toList();

// if they are very close to each other and have the same name, only one is chosen (at random)
var deduplicatedStops = ListUtils
.distinctByKey(
stops,
sl -> new DeduplicationKey(sl.getName(), sl.getCoordinate().roundToApproximate100m())
)
.flatMap(sl -> this.map(sl).stream());
var stations = stopLocationsGroups.stream().map(this::map);
.stream()
.flatMap(s -> this.map(s).stream())
.toList();
var stations = stopLocationsGroups.stream().map(this::map).toList();

return Stream.concat(stops, stations);
return Iterables.concat(deduplicatedStops, stations);
}

StopCluster map(StopLocationsGroup g) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package org.opentripplanner.framework.collection;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Function;

public class ListUtils {

Expand All @@ -13,4 +17,28 @@ public class ListUtils {
public static <T> List<T> combine(Collection<T>... lists) {
return Arrays.stream(lists).flatMap(Collection::stream).toList();
}

/**
* Take a collection and a {@code keyExtractor} to remove duplicates where the key to be compared
* is not the entity itself but a field of it.
* <p>
* Note: Duplicate check is based on equality not identity.
*/
public static <T> List<T> distinctByKey(
Collection<T> original,
Function<? super T, ?> keyExtractor
) {
Set<Object> seen = new HashSet<>();
var ret = new ArrayList<T>();

original.forEach(elem -> {
var key = keyExtractor.apply(elem);
if (!seen.contains(key)) {
seen.add(key);
ret.add(elem);
}
});

return ret;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import java.util.List;
import org.opentripplanner.astar.spi.SkipEdgeStrategy;
import org.opentripplanner.astar.spi.TraverseVisitor;
import org.opentripplanner.framework.lang.PredicateUtils;
import org.opentripplanner.framework.collection.ListUtils;
import org.opentripplanner.street.model.edge.Edge;
import org.opentripplanner.street.model.vertex.TransitStopVertex;
import org.opentripplanner.street.model.vertex.Vertex;
Expand Down Expand Up @@ -42,7 +42,7 @@ public void visitEnqueue() {}
* @return A de-duplicated list of nearby stops found by this visitor.
*/
public List<NearbyStop> stopsFound() {
return stopsFound.stream().filter(PredicateUtils.distinctByKey(ns -> ns.stop)).toList();
return ListUtils.distinctByKey(stopsFound, ns -> ns.stop);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,30 @@ void combine() {
var combined = ListUtils.combine(List.of(1, 2, 3), List.of(5, 6, 7));
assertEquals(List.of(1, 2, 3, 5, 6, 7), combined);
}

private static String makeHello() {
return new String("HELLO");
}

@Test
void distinctByKey() {
var first = new Wrapper(10, makeHello());
var second = new Wrapper(20, makeHello());
var third = new Wrapper(20, "HI");

// the first and second elements have equal values for the "string" property but different
// values for the "i" property
var duplicates = List.of(first, second, third);

// with a normal stream().distinct() or Set.copyOf no item would be removed but when
// we tell the deduplication logic to only look at a single property those with equal
// "string" values are de-duplicated
var deduplicated = ListUtils.distinctByKey(duplicates, w -> w.string);

// as a result the second element is removed from the result as its "string" value is
// equal to the first element's
assertEquals(List.of(first, third), deduplicated);
}

private record Wrapper(int i, String string) {}
}

This file was deleted.

Loading