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

Unify the handling of forward and backward search #6213

Closed
miklcct opened this issue Oct 30, 2024 · 1 comment
Closed

Unify the handling of forward and backward search #6213

miklcct opened this issue Oct 30, 2024 · 1 comment

Comments

@miklcct
Copy link
Contributor

miklcct commented Oct 30, 2024

Currently, different classes have very inconsistent handling between forward search and reverse search. Some use a SearchDirection class, some use a boolean flag, some use separate methods, and some even use separate classes. A boolean flag is bad for readability (does true mean forward and reverse), while separate methods and separate classes result in code duplication with the same logic where only the direction differs, and results in bugs such as #6102 when one of the copies is changed.

Therefore, I propose to move the SearchDirection from Raptor into the util package (#6209) and adopted across the whole code base, which provides a type safe way to control the directionality of searches. This enum should be passed all the way from the request as deep as necessary.

@miklcct miklcct changed the title Unify the handling of forward / departure and backward / arrival search Unify the handling of forward and backward search Oct 31, 2024
@t2gran
Copy link
Member

t2gran commented Nov 5, 2024

We have different things because they are different, and the context is also different. The arriveBy is a USER input witch together with the time tells what the user want for the FIRST request. Note! that when the user page the arriveBy is not longer valid. So in the itinerary filter and in the paging we use SortOrder and PageType to track the needed actions.

SearchDirection in the Raptor api should only be used when talking to Raptor - it has nothing to do with arriveBy. Note also that FORWARD means searching from origin to destination with positive time increments, while REVERSE mean searching from destination to origin with negative time increments. To search in reverse in Raptor is today only used to produce heuristics.

In OTP 1 we presented the best set of itineraries departing after or arriving before a specific time - we do not recommend doing that any more. If you do not know exactly what the user want, presenting a timetable of all pareto-optimal results within a search-window is much better. timetableView is default on in the request. Timetable-view also work well with paging, the old style does not.

In the future when designing a new route query we might use an enum for defartAfter/arriveBefore or we will use better names for the time - e.g. use earliestDepartureTime or latestArrivalTime.

I also wrote about this in #6142.

@miklcct miklcct closed this as not planned Won't fix, can't repro, duplicate, stale Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants