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

Tighter typed Ids #180

Open
antoine-de opened this issue Dec 29, 2018 · 3 comments
Open

Tighter typed Ids #180

antoine-de opened this issue Dec 29, 2018 · 3 comments

Comments

@antoine-de
Copy link
Contributor

The Collectionand Idx are really great to have lots of statically checked stuff.

The transit model has a lot of links between the objects, and for the moment all those links are Strings.
I think navitia_model could be even better if those String ids were also typed.

For example the stop_area_id in the StopPoint could be a Identifier<StopArea> instead of a String.

We could then also remove the get_idx/get method in CollectionWithId<T> to take an &Identifier<T> instead of a &str.

We would also need to find a nice name for this identifier. I don't think we can use the obvious Id because it's already a Trait. Identifier ? Link ? TypedId ? TId ? any other thing ? another option would be to rename the Id trait (maybe to HasId ?)

What do you think ?

@datanel
Copy link
Member

datanel commented Dec 31, 2018

navitia_model could be even better if those String ids were also typed.

why? for semantic?

@antoine-de
Copy link
Contributor Author

for semantic but mainly it would make some bugs impossible, something like:

collections.stop_points.get(my_stop_point.stop_area_id)

we will get a nice build error saying that we are mixing StopPoint and StopArea

@prhod
Copy link
Contributor

prhod commented Jan 2, 2019

I think it's a good idea, but i have concern about the code complexity it would imply.

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

3 participants