Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -26,8 +26,11 @@ the License, or (at your option) any later version.
import org.opentripplanner.routing.graph.Edge;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.routing.graph.Vertex;
import org.opentripplanner.routing.impl.DurationComparator;
import org.opentripplanner.routing.impl.PathComparator;
import org.opentripplanner.routing.request.BannedStopSet;
import org.opentripplanner.routing.spt.DominanceFunction;
import org.opentripplanner.routing.spt.GraphPath;
import org.opentripplanner.routing.spt.ShortestPathTree;
import org.opentripplanner.util.DateUtils;
import org.slf4j.Logger;
Expand All @@ -36,6 +39,7 @@ the License, or (at your option) any later version.
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -268,7 +272,7 @@ public class RoutingRequest implements Cloneable, Serializable {

/** Set of preferred agencies by user. */
public HashSet<String> preferredAgencies = new HashSet<String>();

/**
* Penalty added for using every route that is not preferred if user set any route as preferred. We return number of seconds that we are willing
* to wait for preferred route.
Expand Down Expand Up @@ -441,6 +445,9 @@ public class RoutingRequest implements Cloneable, Serializable {
/** Whether to apply the ellipsoid->geoid offset to all elevations in the response */
public boolean geoidElevation = false;

/** Which path comparator to use */
public String pathComparator = null;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot any places where pathComparator is set. I think a comment on how this is used(set to somethingother null) should be documented. Is the plan to integrate with some kind of configuration later, routing request parameter, or just fork the code and inject a different strategy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not set explicitly in the code, but if you include configuration in router-config.json, it will get set automatically, e.g.

// router-config.json
{
  "routingDefaults": { 
     "pathComparator":"duration" 
   }
}

This happens via the existing config-setting code:

this.defaultRoutingRequest = scraper.scrape(routingDefaultsNode);

I'll add code to allow this to be set via an API parameter as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the clarification. I think your solution is the best, it might be confusing to have "config" params only.

/** Saves split edge which can be split on origin/destination search
*
* This is used so that TrivialPathException is thrown if origin and destination search would split the same edge
Expand Down Expand Up @@ -632,12 +639,12 @@ public void setOtherThanPreferredRoutesPenalty(int penalty) {
if(penalty < 0) penalty = 0;
this.otherThanPreferredRoutesPenalty = penalty;
}

public void setUnpreferredAgencies(String s) {
if (s != null && !s.equals(""))
unpreferredAgencies = new HashSet<String>(Arrays.asList(s.split(",")));
}

public void setUnpreferredRoutes(String s) {
if (s != null && !s.equals(""))
unpreferredRoutes = RouteMatcher.parse(s);
Expand Down Expand Up @@ -669,7 +676,7 @@ public void setBannedStopsHard(String s) {
bannedStopsHard = StopMatcher.emptyMatcher();
}
}

public void setBannedAgencies(String s) {
if (s != null && !s.equals(""))
bannedAgencies = new HashSet<String>(Arrays.asList(s.split(",")));
Expand Down Expand Up @@ -721,7 +728,7 @@ public void setDateTime(String date, String time, TimeZone tz) {
setDateTime(dateObject);
}

public int getNumItineraries() {
public int getNumItineraries() {
if (modes.isTransit()) {
return numItineraries;
} else {
Expand Down Expand Up @@ -757,14 +764,14 @@ public void setIntermediatePlacesFromStrings(List<String> intermediates) {
intermediatePlaces.add(GenericLocation.fromOldStyleString(place));
}
}

/** Clears any intermediate places from this request. */
public void clearIntermediatePlaces() {
if (this.intermediatePlaces != null) {
this.intermediatePlaces.clear();
}
}

/**
* Returns true if there are any intermediate places set.
*/
Expand Down Expand Up @@ -862,7 +869,7 @@ public void setRoutingContext(Graph graph, Edge fromBackEdge, Vertex from, Verte
this.rctx = new RoutingContext(this, graph, from, to);
this.rctx.originBackEdge = fromBackEdge;
}

public void setRoutingContext(Graph graph, Vertex from, Vertex to) {
setRoutingContext(graph, null, from, to);
}
Expand Down Expand Up @@ -1232,4 +1239,12 @@ public void canSplitEdge(StreetEdge edge) {
}

}

public Comparator<GraphPath> getPathComparator(boolean compareStartTimes) {
if ("duration".equals(pathComparator)) {
return new DurationComparator();
}
return new PathComparator(compareStartTimes);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* This program is free software: you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public License
as published by the Free Software Foundation, either version 3 of
the License, or (at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */

package org.opentripplanner.routing.impl;

import org.opentripplanner.routing.spt.GraphPath;

import java.util.Comparator;

public class DurationComparator implements Comparator<GraphPath> {

@Override
public int compare(GraphPath o1, GraphPath o2) {
return o1.getDuration() - o2.getDuration();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public List<GraphPath> getPaths(RoutingRequest options) {
LOG.debug("we have {} paths", paths.size());
}
LOG.debug("END SEARCH ({} msec)", System.currentTimeMillis() - searchBeginTime);
Collections.sort(paths, new PathComparator(options.arriveBy));
Collections.sort(paths, options.getPathComparator(options.arriveBy));
return paths;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* This program is free software: you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public License
as published by the Free Software Foundation, either version 3 of
the License, or (at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
package org.opentripplanner.routing.impl;

import org.junit.Test;
import org.opentripplanner.routing.spt.GraphPath;

import java.util.Arrays;
import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class PathComparatorTest {

GraphPath a = mockGraphPath(5, 7);

GraphPath b = mockGraphPath(0, 8);

GraphPath c = mockGraphPath(9, 12);

private List<GraphPath> paths = Arrays.asList(a, b, c);

@Test
public void testPathComparator() {
paths.sort(new PathComparator(false));
assertEquals(paths, Arrays.asList(a, b, c));
}

@Test
public void testPathComparatorArriveBy() {
paths.sort(new PathComparator(true));
assertEquals(paths, Arrays.asList(c, a, b));
}

@Test
public void testDurationComparator() {
paths.sort(new DurationComparator());
assertEquals(paths, Arrays.asList(a, c, b));
}


private GraphPath mockGraphPath(long startTime, long endTime) {
GraphPath path = mock(GraphPath.class);
when(path.getStartTime()).thenReturn(startTime);
when(path.getEndTime()).thenReturn(endTime);
when(path.getDuration()).thenReturn((int) (endTime - startTime));
return path;
}
}