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

feat(RouteSegment): Break into alerting / non-alerting segments #118

Merged
merged 4 commits into from
Apr 10, 2024

Conversation

KaylaBrady
Copy link
Collaborator

Summary

Ticket: Break route shape segments into alerting / non-alerting segments

What is this PR for?

This adds functionality to break a route segment into a list of alerting / non-alerting segments given the state of alerts.

This will be used for formatting alerting segments differently on the map with an approach similar to how StopSourceGenerator used a map of alertsByStop

Testing

What testing have you done?
Added unit tests

@KaylaBrady KaylaBrady requested a review from a team as a code owner April 8, 2024 18:10
@KaylaBrady KaylaBrady requested a review from BrandonTR April 8, 2024 18:10
@@ -3,22 +3,140 @@ package com.mbta.tid.mbta_app.model
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

interface IRouteSegment {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding as an interface since Kotlin doesn't allow extending data classes

@KaylaBrady KaylaBrady force-pushed the kb-alerting-segments branch from 24a8262 to d65dc17 Compare April 9, 2024 20:47
Copy link
Contributor

@BrandonTR BrandonTR left a comment

Choose a reason for hiding this comment

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

A couple small nitpicks but otherwise lgtm!

.toSet()
routes = routes.plus(sourceRouteId)

var hasServiceAlert: Boolean =
Copy link
Contributor

Choose a reason for hiding this comment

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

We could make this a val instead of a var

routes = routes.plus(sourceRouteId)

var hasServiceAlert: Boolean =
alertsByStop[stopId]!!.serviceAlerts.any { alert ->
Copy link
Contributor

Choose a reason for hiding this comment

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

I typically prefer avoiding the double bang operator if we can. It seems like it would be clean enough to just do:

                    val hasServiceAlert: Boolean? =
                        alertsByStop[stopId]?.serviceAlerts?.any { alert ->
                            alert.anyInformedEntity { informedEntity ->
                                informedEntity.route != null &&
                                    routes.contains(informedEntity.route)
                            }
                        }
                    hasServiceAlert == true

@KaylaBrady KaylaBrady merged commit 75c0f57 into main Apr 10, 2024
5 checks passed
@KaylaBrady KaylaBrady deleted the kb-alerting-segments branch April 10, 2024 20:38
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