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

expose ConnectionQuery::dateType (time arrival or departure) #66

Closed
benib opened this issue May 1, 2012 · 9 comments
Closed

expose ConnectionQuery::dateType (time arrival or departure) #66

benib opened this issue May 1, 2012 · 9 comments

Comments

@benib
Copy link
Contributor

benib commented May 1, 2012

The dateType for the ConnectionQuery should be exposed. This is mentioned in issue #47. Currently it is hard coded to be a departure time.
How should this be exposed. I think a dateType variable which can be set to 1 or 0 is not as clear as it should be.
What about isDepartureTime?

@fabian
Copy link
Member

fabian commented May 2, 2012

Hmm, yes I guess adding a flag is better than adding a second time parameter like departureTime, as that's how it usually will be used.

How about timeType with values departure and arrival?

@1tchy
Copy link

1tchy commented May 2, 2012

I think passing a parameter isDepartureTime=0 is clear to be the arrival time. And isDepartureTime=1 is clear to be the departure.

@colinfrei
Copy link
Member

I'd rather name the argument so that it's clear instead of adding an additional parameter.

So have a departuretime and an arrivaltime argument, and throw a 400 error if you get both.

@benib
Copy link
Contributor Author

benib commented May 2, 2012

@colinfrei: this would mean we have depdate, deptime, arrdate and arrtime.
I think a parameter isArrivalTime would be the best, as this defaults to 0.

@colinfrei
Copy link
Member

The downside is that it makes it more magic. in my eyes arguments of which it's obvious what they do are always better than arguments where it's not obvious, even if that means having more arguments.

For example you'd still have the situation where it is like it is now, and you're not sure if arrival or departure is meant.

@benib
Copy link
Contributor Author

benib commented May 2, 2012

I agree about the magic but i think that in this case, it would be easier for everybody to have some magic.
Think about a simple form where you can set the date/time and select departure or arrival (as the sbb.ch form). then you can handle the date/time the same way, and just add the isArrivalTime if arrival is selected.
Thats also the way, the xmlfahrplan API handles this, so we don't have to fiddle around internally.

Defaulting to departure is the way every fahrplan application handles this. so this is clear for me.

@1tchy
Copy link

1tchy commented May 2, 2012

Whether the parameter is an isArrivalTime or isDepartureTime doesn't really matter for me. However, I agree with benib that there should be one extra parameter and not two changed and two invented ones. And I agree, it should default to departure. So it's indeed smarter to have the default isArrivalTime=0.

@akuendig
Copy link

akuendig commented May 6, 2012

+1 This would be a great addition

@fabian
Copy link
Member

fabian commented May 7, 2012

PR merged.

@fabian fabian closed this as completed May 7, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants