public-transport/fpti-js

Some ideas

Closed this issue · 12 comments

ialokim commented

First, thanks a lot @juliuste for working on this draft! I've had a quick look and in general it looks quite reasonable and good to me. However, I wanted to make some little suggestions:

  • By nature, not all PT APIs cover all of the features and nor will their respective client libraries. In my opinion, it would be very useful to introduce a new static method, which specifies the different features this specific client supports. I'm not sure yet about how fine-grained it should report the features (so, eg. only departures: yes/no or even which of the opt of each method are supported), but I think this would be very helpful while developing projects that want to access different clients, while being aware of the specific features.
  • For stations/stops/regions.nearby, I would opt for adding another required option called results to specify the maximum number of results returned (as in journeys or stopovers).
  • For stopovers, I would add a required (?) option whenRepresents as in journeys to specify if we want to query departures or arrivals.
  • To be consistent with stations.search() and stations.nearby() I suggest renaming stations() to stations.all().
  • For journeys I think it would be quite useful to add products to the (required) options as e.g. supported by the hafas-client or even name it modes sticking to the vocab used in FTPF.
juliuste commented

Thank you very much for your feedback 🙂

  • By nature, not all PT APIs cover all of the features and nor will their respective client libraries. In my opinion, it would be very useful to introduce a new static method, which specifies the different features this specific client supports. I'm not sure yet about how fine-grained it should report the features (so, eg. only departures: yes/no or even which of the opt of each method are supported), but I think this would be very helpful while developing projects that want to access different clients, while being aware of the specific features.

For a general overview about which methods are supported, you could just check the main module object's attributes, e.g. if module.journeys exists, there will be a journeys method. Not sure if this is enough, though, we could - for example - require methods to have an additional opt key (e.g. module.journeys.opt that would contain an object with all supported options and their feature descriptions, e.g. specialModule.journeys.opt could give

{
    when: "Date of the journey",
    results: "Number of results returned",
    specialOption: "Module-Specific option description"
}

Do you think this would be enough for developers?

  • For stations/stops/regions.nearby, I would opt for adding another required option called results to specify the maximum number of results returned (as in journeys or stopovers).

👍 Same probably for stations/stops/regions.search.

  • For stopovers, I would add a required (?) option whenRepresents as in journeys to specify if we want to query departures or arrivals.

This makes sense, however I'm also not sure about the option being required (from a technical point of view, since some modules might not support that). I'm not quite sure if we shouldn't also make journeys.opt.whenRepresents optional instead. (And just always default to departures if the option is not enabled).

  • To be consistent with stations.search() and stations.nearby() I suggest renaming stations() to stations.all().

I wouldn't have a problem with that, @derhuerst opinions?

  • For journeys I think it would be quite useful to add products to the (required) options as e.g. supported by the hafas-client or even name it modes sticking to the vocab used in FTPF.

I see why this would make sense, however I'm not sure if we shouldn't add products to the FPTF spec first, then.

  • 👍1
ialokim commented

Do you think this would be enough for developers?

That sounds very good!

👍 Same probably for stations/stops/regions.search.

You are right, sounds good.

I'm not quite sure if we shouldn't also make journeys.opt.whenRepresents optional instead. (And just always default to departures if the option is not enabled)

Would be fine for me.

I see why this would make sense, however I'm not sure if we shouldn't add products to the FPTF spec first, then.

Aren't they kind of supported through modes?

juliuste commented

Aren't they kind of supported through modes?

As far as I know, products describe stuff like express, nationalExpress, regional etc., which mostly share the train mode. Just filtering for trains/buses/… (by mode) is probably not too useful in most cases, right? Or maybe it is, I don't know 😄

We also have the reserved subMode field, we should discuss the difference between this and products, and maybe include one or both of them as optional in FPTF.

derhuerst commented

we could - for example - require methods to have an additional opt key (e.g. module.journeys.opt that would contain an object with all supported options and their feature descriptions

Let's call it defaults. opt indicates that you're supposed to change them (globally).

For stations/stops/regions.nearby, I would opt for adding another required option called results to specify the maximum number of results returned (as in journeys or stopovers).

👍 Same probably for stations/stops/regions.search.

Why not make it optional? I'm not sure yet if it makes sense for all API clients to have a results opt.

For stopovers, I would add a required (?) option whenRepresents as in journeys to specify if we want to query departures or arrivals.

I'm not quite sure if we shouldn't also make journeys.opt.whenRepresents optional instead. (And just always default to departures if the option is not enabled).

Let's pick departureAfter and arrivalBefore instead of when and whenRepresents.

For journeys I think it would be quite useful to add products to the (required) options as e.g. supported by the hafas-client or even name it modes sticking to the vocab used in FTPF.

@ialokim Keep in mind that products is different from modes.

A mode is intended to be a general, coarse description of the type of vehicle (e.g. train).

A product is a precise description of a means of transport (e.g. s-bahn), in local context, and may express pricing models (regionalexpress vs. s-bahn even though they might be the same type of vehicle).

edit: @juliuste already explained this.

  • 👍1
ialokim commented

Let's call it defaults. opt indicates that you're supposed to change them (globally).

Okay, I see that point, but defaults does not suit very well either in my opinion: The idea was to get a list of the features (options) supported by the specific method, paired with an explanation. So either it should really return a list of supported options with their default value, then it would make sense to name it defaults, or we could use the idea from @juliuste, but should name it differently then, maybe features or supports?

Why not make it optional? I'm not sure yet if it makes sense for all API clients to have a results opt.

I would go for requiring it here to have a wider standard, clients for APIs that doesn't support this option could only return the first x results when results has been set to x.

Let's pick departureAfter and arrivalBefore instead of when and whenRepresents.

I quite like the idea of using the same naming as specified in FTPF, so I would vote for when and whenRepresents. Also, departureAfter and arrivalBefore should then be mutually exclusive while one of them should remain required, which is a bit less straightforward in my opinion.

@ialokim Keep in mind that products is different from modes.

Okay, I was not aware of that difference, but after having a quick read on FPTF#4, I agree with @juliuste that this should be first discussed for FTPF and then added here accordingly.

  • 👍2
juliuste commented

Let's call it defaults. opt indicates that you're supposed to change them (globally).

Okay, I see that point, but defaults does not suit very well either in my opinion: The idea was to get a list of the features (options) supported by the specific method, paired with an explanation. So either it should really return a list of supported options with their default value, then it would make sense to name it defaults, or we could use the idea from @juliuste, but should name it differently then, maybe features or supports?

@derhuerst was probably also referring to the proposed options "dictionary" (with descriptions), so let's just call it features, then? 🙃

Why not make it optional? I'm not sure yet if it makes sense for all API clients to have a results opt.

I would go for requiring it here to have a wider standard, clients for APIs that doesn't support this option could only return the first x results when results has been set to x.

I agree with @ialokim here. results its also really easy to implement. I see more issues arising with the interval attribute since libraries might have to spawn lots of requests here, but I felt that it was really important to have this option supported by every module because the very unstandardized handling of time intervals by different endpoints was one of the main pitfalls when I was building "end-user" software (😄) with our modules (like Bahn-Guru or pricemap.eu).

Let's pick departureAfter and arrivalBefore instead of when and whenRepresents.

I quite like the idea of using the same naming as specified in FTPF, so I would vote for when and whenRepresents. Also, departureAfter and arrivalBefore should then be mutually exclusive while one of them should remain required, which is a bit less straightforward in my opinion.

I kind of agree with both of you here 🙈. In theory @derhuerst|s proposal seems more elegant to me, but I also share @ialokim|s concerns.

This might seem dumb, but I worry about departureAfter being too long/complicated as an option name. Even now using when, people might wonder why the journey date (e.g.) isn't just a normal required parameter. But at least when "skipping through" the opt attributes, they immediately understand how to submit a custom date. I know that this is a weird argument to make, but I feel like that departureAfter "sounds" way more complex than when at first sight, especially if you never thought about any of these questions and only want to use a module for the basic use case of journeys between two stations at a given time. 😄

Also, APIs (at least those that I have covered) are significantly more likely to only support the "most default" use case departureAfter, and not arrivalBefore, which would justify having a slightly less easy-to-understand way to express arrivalBefore (by using when and whenRepresents) in favour of a less complicated way to express departureAfter (by only using when).

However, at the same time when on its own doesn't clearly indicate what case (before/after) we are talking about which is especially important for module developers. I have had multiple bugs because I hadn't spent attention to what the endpoint actually returns for a given when, as long as it was roughly on the same day as when. Using departureAfter and arrivalBefore makes this much more obvious for the developer side IMHO.

All in all, I probably slightly favour using when and whenRepresents, but if someone heavily prefers the other proposal, I'd also not disagree.

derhuerst commented

Why not make it optional? I'm not sure yet if it makes sense for all API clients to have a results opt.

I would go for requiring it here to have a wider standard, clients for APIs that doesn't support this option could only return the first x results when results has been set to x.

I'd be very careful with this approach. One of the reasons why @juliuste and I set out to create yet another format/standard is because the existing standards were to bloated/strict/inflexible.

derhuerst commented

I quite like the idea of using the same naming as specified in FTPF, so I would vote for when and whenRepresents.

In hafas-client@2 they are named like this to stay-backwards compatible. In hafas-client@3 they are already renamed.

Also, departureAfter and arrivalBefore should then be mutually exclusive while one of them should remain required, which is a bit less straightforward in my opinion.

Most trains, buses and planes APIs that I know of have these as exclusive variants.

derhuerst commented

This might seem dumb, but I worry about departureAfter being too long/complicated as an option name. Even now using when, people might wonder why the journey date (e.g.) isn't just a normal required parameter. But at least when "skipping through" the opt attributes, they immediately understand how to submit a custom date.

Fair point. Maybe a different name helps.

I know that this is a weird argument to make, but I feel like that departureAfter "sounds" way more complex than when at first sight, especially if you never thought about any of these questions and only want to use a module for the basic use case of journeys between two stations at a given time. 😄

I think this is part of the problem. While we should recognize that "journey from A to B starting at 11 am" is what people usually expect, they often want "journey from A to B arriving at 12 am", but don't expect the journey planning algorithm to support this.

However, at the same time when on its own doesn't clearly indicate what case (before/after) we are talking about which is especially important for module developers. I have had multiple bugs because I hadn't spent attention to what the endpoint actually returns for a given when, as long as it was roughly on the same day as when. Using departureAfter and arrivalBefore makes this much more obvious for the developer side IMHO.

⚠️

derhuerst commented

re when/departureAfter/arrivalBefore:

What about handling when as a synonym to departureAfter? It would fail if you provided when & arrivalBefore at the same time (and if you used when and departureAfter), but work otherwise.

juliuste commented

What about handling when as a synonym to departureAfter? It would fail if you provided when & arrivalBefore at the same time (and if you used when and departureAfter), but work otherwise.

Sounds good to me, I implemented this (for now, since nobody complained). Will add the other proposals later this week, if you don't mind 🙂

juliuste commented

Ok, I think I implemented everything proposed here, so I'll close this issue for now. Feel free to re-open (or to create separate issues) if I forgot anything.