Skip to content

Conversation

Dlurak
Copy link
Collaborator

@Dlurak Dlurak commented Oct 3, 2024

Description

The public transport categories are now more reliable by changing these things:

  • Actually using routes and route_masters and not only route_master relations
  • Using highspeed=yes tags
  • Accepting both service=high_speed and service=highspeed (372 uses for the correct high_speed and 18 for highspeed)

Example links

Frankfurt central station

Screenshots

Old New
image image

Copy link

vercel bot commented Oct 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
osmapp ✅ Ready (Inspect) Visit Preview Oct 3, 2024 8:47am

@Dlurak Dlurak enabled auto-merge (squash) October 3, 2024 08:45
@Dlurak Dlurak merged commit 1f45177 into zbycz:master Oct 3, 2024
2 checks passed
Comment on lines 29 to +50
}
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')
);
};
Copy link
Owner

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.

Copy link
Collaborator Author

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:

  1. tags.service
  2. routes[].tags.service
  3. tags.highspeed=yes
  4. routes[].tags=highspeed=yes
  5. tags.route
  6. routes[].tags.route
  7. ...

Copy link
Owner

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.

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

Successfully merging this pull request may close these issues.

2 participants