Skip to content

🎨 Return Station objects in getLatestArrivals #3565

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

Merged

Conversation

MartinNemi03
Copy link
Contributor

Originally meant to have this add areas to station history (thus having area in autocomplete before typing a destination) and even though it seems that attribute would just need accessing in order to write itself into the output object, it seemed kinda iffy..

So at least it now returns Station objects as commented in TODO.

->limit($maxCount)
->pluck('station_id')
->map(function (int $id) {
return Station::where('id', $id)->first();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would result in an additional $maxCounts SQL queries, which would not scale well on the production instance. :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair and thanks for letting me know, this isn't the best solution in regards to performance as I wasn't really looking into how many queries that would add. I'll turn this into a draft PR and try to approach it in a different way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went ahead and just made it so it maps the values into a new Station instance.

@MartinNemi03 MartinNemi03 marked this pull request as draft May 27, 2025 14:14
@MartinNemi03 MartinNemi03 marked this pull request as ready for review May 28, 2025 19:51
@MartinNemi03 MartinNemi03 force-pushed the latest-arrivals-stations branch from f6480c1 to 170f631 Compare May 28, 2025 19:54
@MartinNemi03 MartinNemi03 requested a review from MrKrisKrisu May 30, 2025 15:35
@HerrLevin HerrLevin merged commit b412c57 into Traewelling:develop Jun 2, 2025
6 checks passed
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.

3 participants