feat: add builder to OtpApiClient#131
feat: add builder to OtpApiClient#131leonardehrenfried merged 3 commits intoopentripplanner:mainfrom
OtpApiClient#131Conversation
| this((BuilderImpl) OtpApiClient.builder().baseUri(baseUrl).timeZone(timezone)); | ||
| } | ||
|
|
||
| private OtpApiClient(final BuilderImpl b) { |
There was a problem hiding this comment.
If this method is private, how do you use the builder then?
There was a problem hiding this comment.
Through OtpApiClient::builder, so the usage would be
OtpApiClient client = OtpApiClient.builder().baseUri(...).timeZone(...).build()
| } | ||
|
|
||
| private OtpApiClient(final BuilderImpl b) { | ||
| final var zoneId = Objects.requireNonNullElse(b.zoneId, ZoneId.systemDefault()); |
There was a problem hiding this comment.
The zone id must be provided otherwise we get confusing results.
Having said that, i actually want to get rid of it completely because in newer OTP versions, it's no longer needed. The zone is part of the response.
There was a problem hiding this comment.
Agree, I added it as a required parameter
|
Maybe write a test or some Markdown as a bit of documentation. |
jeeeesper
force-pushed
the
add-client-builder
branch
from
9f28c9b to
327edea
Compare
|
I wrote some (very basic) tests and a small addition to the README file, and addressed your comments in code. I'm not sure whether my tests are good where they are, through, or whether I should move them to |
|
Thanks for this. I think having custom headers are a frequent use case. If you want you can add a special method to the builder for it. |
2 participants
This, for me, came up because I wanted to specify the full URL and an authentication token. For example, the Routing API for the Helsinki region is available at
https://api.digitransit.fi/routing/v2/hsl/gtfs/v1, where the path is not the default path configured in the client.I'm happy to change the implementation or discuss about other approaches, this seemed like the most obvious solution to me.