-
-
Notifications
You must be signed in to change notification settings - Fork 29
FeaturePanel: Reliable public transport cateegories #625
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
FeaturePanel: Reliable public transport cateegories #625
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
} | ||
const altElement = routes.find(({ tags }) => tags[key]); | ||
if (altElement) { | ||
return altElement[key]; | ||
return altElement.tags[key]; | ||
} | ||
return undefined; | ||
}; | ||
|
||
const getService = (tags: Record<string, string>, routes: WithTags[]) => { | ||
const getVal = (key: string) => getTagValue(key, tags, routes); | ||
const serviceTagValue = getTagValue('service', tags, routes); | ||
const serviceTag = | ||
serviceTagValue === 'highspeed' ? 'high_speed' : serviceTagValue; | ||
const isHighspeed = getVal('highspeed') === 'yes'; | ||
|
||
return ( | ||
serviceTag || | ||
(isHighspeed && 'high_speed') || | ||
getVal('route') || | ||
getVal('route_master') | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please explain what this should do? I find the code very hard to read. I guess there could be two things mixed together - like getting a tag from relations tags
and getting it from the routes[].tags
. When is which scenario used?
Maybe then we can find more suitable code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It always looks for the relations tags
and then routes[].tags
, so like this:
tags.service
routes[].tags.service
tags.highspeed=yes
routes[].tags=highspeed=yes
tags.route
routes[].tags.route
- ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understand now. So, the semantics of getTagValue()
is in reality get-tag-or-get-tag-on-child-route
. I think this function does two things which are unrelated.
More readable code for me would be:
const getValue = (
key: string,
tags: Record<string, string>,
) => {
if (tags[key]) {
return tags[key];
}
return undefined;
};
const getValueOnAnyRoute = (
key: string,
routes: WithTags[],
) => {
const altElement = routes.find(({ tags }) => tags[key]);
if (altElement) {
return altElement.tags[key];
}
return undefined;
};
const getService = (tags: Record<string, string>, routes: WithTags[]) => {
const getVal = (key: string) => getValue(key, tags) ?? getValueOnAnyRoute(key, routes);
Do you get the difference? It is subtle, but code like this makes it easier for next reader to get the meaning. They can only read the const getVal
line to get the full meaning, without opening the complicated (boilerplate) accessing code.
Of course, this is more like pseudo-code highlight the idea, it could be probably refactored much more.
If you want, you can edit it in some followup.
Description
The public transport categories are now more reliable by changing these things:
highspeed=yes
tagsservice=high_speed
andservice=highspeed
(372 uses for the correcthigh_speed
and 18 forhighspeed
)Example links
Frankfurt central station
Screenshots