Conversation
📝 WalkthroughWalkthroughAdds distance- and open-status-based sorting to several facility lists in HomeViewModel, filters printers with blank locations, exposes a public numerical distance helper, and adds a TimeUtils import to an ecosystem bottom sheet component. Changes
Sequence Diagram(s)sequenceDiagram
participant HomeVM as HomeViewModel
participant LocRepo as LocationRepository\n(currentLocation)
participant Repo as FacilityRepos\n(printers, eateries, gyms, libraries)
participant TimeU as TimeUtils
rect rgba(200,230,255,0.5)
HomeVM->>LocRepo: collect currentLocation
HomeVM->>Repo: request ApiResponses for each facility type
Repo-->>HomeVM: ApiResponse.Success(list) / other
end
rect rgba(220,255,220,0.5)
HomeVM->>HomeVM: numericalDistanceToPlace(lat, lon)
HomeVM->>TimeU: getOpenStatus(hours)
HomeVM-->>HomeVM: sortApiResponse -> apply open-first (if eateries/gyms), filter printers with blank location, then sort by distance
HomeVM-->>UI: emit sorted StaticPlaces
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt`:
- Around line 548-552: The eateries list currently sorts by a string distance
(distanceStringToPlace) and a fragile substring check on
operatingHoursToString(...).contains("open"), causing incorrect order; replace
these with a stable numeric distance comparator and an explicit open-status
check: compute a numeric distance (e.g., getDistanceMeters or a new helper using
latitude/longitude) for each item instead of distanceStringToPlace, and use a
boolean isPlaceOpen(eatingPlace.operatingHours()) or similar function that
inspects structured hours rather than string.contains; then sort
eateriesApiResponse.data with a single sortedWith comparator that first prefers
open items (isOpen true) and then ascending numeric distance so open places
appear before closed and closer places come first.
- Around line 593-595: The sorting is using the string-based distance helper
distanceStringToPlace on staticPlaces.libraries.data; change it to use the
numeric distance helper (e.g., distanceToPlace or another function that returns
a Double/Int) so libraries are sorted numerically by distance before rendering
in the items(...) block inside EcosystemBottomSheetContent.kt; locate the
items(staticPlaces.libraries.data.sortedBy { ... }) call and replace the
string-based comparator with the numeric distance function and ensure
null-safety if the numeric function can return null.
- Around line 491-494: The printers list is being sorted by the string result of
distanceStringToPlace which yields incorrect numeric ordering; change the sort
to use a numeric distance value (e.g., replace sortedBy {
distanceStringToPlace(it.latitude, it.longitude) } with sortedBy {
distanceToPlace(it.latitude, it.longitude) } or compute a numeric distance via
Location.distanceBetween) and keep distanceStringToPlace only for display;
update or add a helper named distanceToPlace(latitude, longitude) if one doesn't
exist and use staticPlaces.printers.data.filter { it.location != "" } as before.
- Around line 442-446: Replace the lexicographic sort using
distanceStringToPlace and the fragile contains("open") check: in the items(...)
call (where gymsApiResponse.data is sorted) use numeric sorting via
calculateDistance(...) (pass the currentLocation and the gym's LatLng or the
utilities' expected params) to sort by ascending distance, and use
getOpenStatus(it.operatingHours()).isOpen for boolean open/closed sorting
(sortedByDescending) instead of operatingHoursToString(...).contains("open").
Remove the distanceStringToPlace usage from the sort and reserve it only for
display.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4940e075-661f-4287-b6dd-f9523691de4d
📒 Files selected for processing (1)
app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt
| items(gymsApiResponse.data | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) } | ||
| .sortedByDescending { operatingHoursToString(it.operatingHours()) | ||
| .contains("open", ignoreCase = true) } | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's already a numeric distance calculation available in the codebase
rg -n "fun.*distance.*Double" --type kotlin
ast-grep --pattern 'fun $NAME($_): Double { $$$ }'Repository: cuappdev/ithaca-transit-compose
Length of output: 248
🏁 Script executed:
# Find the distanceStringToPlace function definition
rg -n "distanceStringToPlace" --type kotlin -A 5Repository: cuappdev/ithaca-transit-compose
Length of output: 31015
🏁 Script executed:
# Search for numeric distance functions that might return Double
rg -n "calculateDistance|getDistance|computeDistance" --type kotlin
rg -n "haversine|euclidean" --type kotlinRepository: cuappdev/ithaca-transit-compose
Length of output: 417
🏁 Script executed:
# Look at the full context of the EcosystemBottomSheetContent.kt file around the sorting logic
cat -n app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt | sed -n '430,460p'Repository: cuappdev/ithaca-transit-compose
Length of output: 1421
🏁 Script executed:
# Check for operating hours utility functions
rg -n "operatingHoursToString|isCurrentlyOpen|operatingHours" --type kotlin | head -20Repository: cuappdev/ithaca-transit-compose
Length of output: 2809
🏁 Script executed:
cat app/src/main/java/com/cornellappdev/transit/util/DistanceUtils.ktRepository: cuappdev/ithaca-transit-compose
Length of output: 465
🏁 Script executed:
# Find the full isOpenAnnotatedStringFromOperatingHours and getOpenStatus functions
rg -A 10 "fun getOpenStatus" --type kotlin
rg -A 5 "fun isOpenAnnotatedStringFromOperatingHours" --type kotlinRepository: cuappdev/ithaca-transit-compose
Length of output: 2745
Sorting by string distance produces incorrect numeric ordering.
distanceStringToPlace returns a String (e.g., "2.5 mi", "10.1 mi"), but lexicographic sorting produces incorrect results. For example, "2.5 mi" > "10.1 mi" lexicographically, causing gyms farther away to appear before closer ones.
Use the existing calculateDistance() utility (which returns Double) for sorting, and reserve the string version for display only.
Additionally, contains("open", ignoreCase = true) is fragile and could match unintended substrings. Use getOpenStatus(operatingHours).isOpen (available in TimeUtils.kt) instead for a proper boolean-based sort.
Suggested approach
// In sorting logic:
items(gymsApiResponse.data
.sortedBy { calculateDistance(currentLocation, LatLng(it.latitude, it.longitude)) }
.sortedByDescending { getOpenStatus(it.operatingHours()).isOpen }
)Where calculateDistance handles numeric distance properly and getOpenStatus().isOpen returns a reliable boolean for sorting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt`
around lines 442 - 446, Replace the lexicographic sort using
distanceStringToPlace and the fragile contains("open") check: in the items(...)
call (where gymsApiResponse.data is sorted) use numeric sorting via
calculateDistance(...) (pass the currentLocation and the gym's LatLng or the
utilities' expected params) to sort by ascending distance, and use
getOpenStatus(it.operatingHours()).isOpen for boolean open/closed sorting
(sortedByDescending) instead of operatingHoursToString(...).contains("open").
Remove the distanceStringToPlace usage from the sort and reserve it only for
display.
There was a problem hiding this comment.
^^ @AbigailLabanok Double check if this is the case and fix
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| val printers = staticPlaces.printers.data.filter { it.location != "" } | ||
| items(printers | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) }) | ||
| { |
There was a problem hiding this comment.
Same string-based distance sorting issue applies here.
The printer list has the same problem: sorting by the string representation of distance will not produce correct numeric ordering.
The filtering logic filter { it.location != "" } looks correct for hiding printers without a location.
Suggested fix pattern
val printers = staticPlaces.printers.data.filter { it.location != "" }
items(printers
- .sortedBy { distanceStringToPlace(it.latitude, it.longitude) })
+ .sortedBy { calculateNumericDistance(it.latitude, it.longitude) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt`
around lines 491 - 494, The printers list is being sorted by the string result
of distanceStringToPlace which yields incorrect numeric ordering; change the
sort to use a numeric distance value (e.g., replace sortedBy {
distanceStringToPlace(it.latitude, it.longitude) } with sortedBy {
distanceToPlace(it.latitude, it.longitude) } or compute a numeric distance via
Location.distanceBetween) and keep distanceStringToPlace only for display;
update or add a helper named distanceToPlace(latitude, longitude) if one doesn't
exist and use staticPlaces.printers.data.filter { it.location != "" } as before.
There was a problem hiding this comment.
^^ Same thing here as before
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the implementation of distanceStringToPlace
rg -n "distanceStringToPlace\|distanceToPlace\|distanceBetween" --type=kt -A5 -B2
Length of output: 26
🏁 Script executed:
#!/bin/bash
# Find the implementation of distanceStringToPlace
rg -n "distanceStringToPlace|distanceToPlace|distanceBetween" -g "*.kt" -A5 -B2
Length of output: 42892
🏁 Script executed:
#!/bin/bash
# Find the implementation of distanceStringIfCurrentLocationExists
rg -n "distanceStringIfCurrentLocationExists\|fun distanceString" -g "*.kt" -A10 -B2
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find HomeViewModel file
fd -e kt HomeViewModel
# Search for distance-related functions in all kt files
rg -n "fun distance" -g "*.kt" -A10 -B2
Length of output: 1810
@AndrewCheung360, confirmed — the same concern applies here. The distanceStringIfCurrentLocationExists function returns strings formatted as " - X.XX mi", which sort lexicographically incorrectly once distances reach 10+ miles (e.g., " - 9.5 mi" would sort after " - 10.0 mi").
That said, given all locations are on or near Cornell's campus in Ithaca, distances will realistically stay well under 10 miles, so the practical impact on sort correctness is negligible. If you'd like a clean fix regardless, the approach would be to expose a numeric distance method from HomeViewModel (wrapping the existing calculateDistance from DistanceUtils.kt) and use it as the sort key while keeping distanceStringToPlace only for display:
// In HomeViewModel
fun numericDistanceToPlace(latitude: Double?, longitude: Double?): Double {
val currentLocationSnapshot = currentLocation.value
return if (currentLocationSnapshot != null && latitude != null && longitude != null) {
calculateDistance(
LatLng(currentLocationSnapshot.latitude, currentLocationSnapshot.longitude),
LatLng(latitude, longitude)
)
} else Double.MAX_VALUE
}Then sort with:
val printers = staticPlaces.printers.data.filter { it.location != "" }
items(printers.sortedBy { numericDistanceToPlace(it.latitude, it.longitude) }) { ... }This would be consistent with the fix for the gym/eatery lists as well. Happy to defer this to a follow-up if that was the agreed approach for the earlier discussion.
| items(eateriesApiResponse.data | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) } | ||
| .sortedByDescending { operatingHoursToString(it.operatingHours()) | ||
| .contains("open", ignoreCase = true) } | ||
| ) { |
There was a problem hiding this comment.
Eateries have the same sorting issues as gyms.
Both the string-based distance sorting and the fragile contains("open") check apply here. This should be fixed consistently across all list types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt`
around lines 548 - 552, The eateries list currently sorts by a string distance
(distanceStringToPlace) and a fragile substring check on
operatingHoursToString(...).contains("open"), causing incorrect order; replace
these with a stable numeric distance comparator and an explicit open-status
check: compute a numeric distance (e.g., getDistanceMeters or a new helper using
latitude/longitude) for each item instead of distanceStringToPlace, and use a
boolean isPlaceOpen(eatingPlace.operatingHours()) or similar function that
inspects structured hours rather than string.contains; then sort
eateriesApiResponse.data with a single sortedWith comparator that first prefers
open items (isOpen true) and then ascending numeric distance so open places
appear before closed and closer places come first.
There was a problem hiding this comment.
🧩 Analysis chain
Let me look at the available utility functions to propose the right fix.
🏁 Script executed:
#!/bin/bash
# Find distance/location utility functions
rg -rn "fun.*[Dd]istance" --type=kotlin -A3
echo "---"
# Find any isOpen or open-status boolean helpers for operating hours
rg -rn "fun.*[Ii]s[Oo]pen\|fun.*[Oo]pen[Ss]tatus\|fun.*[Oo]pening" --type=kotlin -A5
echo "---"
# Find TimeUtils and isOpenAnnotatedStringFromOperatingHours
rg -rn "isOpenAnnotatedString\|TimeUtils" --type=kotlin -A5
echo "---"
# Find DayOperatingHours and operatingHours() model
fd -e kt -e java "DayOperatingHours" --exec cat {}
echo "---"
# Find operatingHours() method
rg -rn "fun operatingHours" --type=kotlin -A10
Length of output: 6118
🏁 Script executed:
#!/bin/bash
# Get full DistanceUtils
fd "DistanceUtils.kt" --exec cat {}
echo "==="
# Get full TimeUtils
fd "TimeUtils.kt" --exec cat {}
echo "==="
# Get HomeViewModel distance string function
rg -n "distanceString\|currentLocation\|LatLng" app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt -A5 -B2
Length of output: 9924
@AndrewCheung360, here's the plan to properly fix both issues in eateryList and gymList:
Issue 1 — Fragile contains("open") check
TimeUtils.getOpenStatus() already returns a structured OpenStatus(isOpen: Boolean, ...). Use it directly instead of parsing the display string:
// Before (fragile)
.sortedByDescending {
operatingHoursToString(it.operatingHours()).contains("open", ignoreCase = true)
}
// After (correct)
.sortedByDescending {
TimeUtils.getOpenStatus(it.operatingHours()).isOpen
}This applies to both eateryList and gymList.
Issue 2 — String-based distance sort
distanceStringToPlace returns a display string (e.g. " - 0.5 mi"). Sorting by this lexicographically is broken — e.g. " - 10.0 mi" would sort before " - 2.0 mi". calculateDistance(from: LatLng, to: LatLng): Double in DistanceUtils.kt exists and returns meters, but current location isn't threaded into these list functions.
The fix requires adding a numeric distance lambda alongside the existing display one:
- Add
numericDistanceToPlace: (Double?, Double?) -> Double?lambda toeateryList,gymList,libraryList,printerList, andBottomSheetFilteredContent - Thread it from
EcosystemBottomSheetContent(passed from ViewModel) - Replace
.sortedBy { distanceStringToPlace(...) }with.sortedBy { numericDistanceToPlace(...) ?: Double.MAX_VALUE }in all four list functions - Replace
.sortedByDescending { operatingHoursToString(...).contains("open") }with.sortedByDescending { TimeUtils.getOpenStatus(it.operatingHours()).isOpen }ineateryListandgymList - Add the lambda construction in
HomeViewModelusingcalculateDistance+currentLocation
Failed to handle agent chat message. Please try again.
| items(staticPlaces.libraries.data | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) } | ||
| ) { |
There was a problem hiding this comment.
Libraries also affected by string distance sorting.
Same fix needed here—use numeric distance for sorting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt`
around lines 593 - 595, The sorting is using the string-based distance helper
distanceStringToPlace on staticPlaces.libraries.data; change it to use the
numeric distance helper (e.g., distanceToPlace or another function that returns
a Double/Int) so libraries are sorted numerically by distance before rendering
in the items(...) block inside EcosystemBottomSheetContent.kt; locate the
items(staticPlaces.libraries.data.sortedBy { ... }) call and replace the
string-based comparator with the numeric distance function and ensure
null-safety if the numeric function can return null.
There was a problem hiding this comment.
🧩 Analysis chain
Let me search the codebase for a numeric distance helper function.
🏁 Script executed:
#!/bin/bash
# Search for distance-related utility functions that return a numeric type
rg -n "fun distance" --type=kt -A3
Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for distance-related utility functions
rg -n "fun distance" -g "*.kt" -A3
# Also search for any function returning Double related to distance/place
rg -n "distanceTo\|distanceToPlace\|getDistance\|calculateDistance\|distanceFrom" -g "*.kt" -A3
Length of output: 585
🏁 Script executed:
#!/bin/bash
# Get the full distance function and surrounding code in HomeViewModel
rg -n "distanceString\|currentLocation\|distanceTo\|Location\." -g "*.kt" -A5 -B2 \
app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt | head -80
# Also check for any utility that computes raw numeric distance
rg -n "fun.*latitude\|fun.*longitude\|haversine\|SphericalUtil\|distanceBetween" -g "*.kt" -A4
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find all Kotlin files
fd -e kt HomeViewModel
# Search broadly for distance-related content
rg -n "distance" -g "*.kt" | grep -v "distanceStringToPlace\|distanceStringIfCurrentLocationExists\|distanceString" | head -40
Length of output: 4190
🏁 Script executed:
#!/bin/bash
cat app/src/main/java/com/cornellappdev/transit/util/DistanceUtils.kt
echo "---HomeViewModel distance section---"
sed -n '375,410p' app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt
Length of output: 1721
@AndrewCheung360, the issue is confirmed and real. Here's the breakdown:
distanceStringToPlace produces strings like " - 9.9 mi" or " - 10.1 mi". Because Kotlin's sortedBy on a String uses lexicographic (character-by-character) ordering, " - 9.9 mi" sorts after " - 10.1 mi" (since '9' > '1'). This means anything beyond 9.x miles will be mis-ordered.
The existing calculateDistance in DistanceUtils.kt already returns a numeric Double, but there's no numeric distance lambda currently threaded into EcosystemBottomSheetContent. The cleanest fix is to add a parallel distanceToPlace: (Double?, Double?) -> Double? lambda for sorting, while keeping distanceStringToPlace for display.
- Add
distanceToPlace: (Double?, Double?) -> Double?parameter toEcosystemBottomSheetContent,BottomSheetFilteredContent, and all four list functions (gymList,eateryList,printerList,libraryList) - Replace string-based
sortedBy { distanceStringToPlace(...) }withsortedBy { distanceToPlace(...) }in all four list functions - Wire the new lambda in
HomeViewModelusingcalculateDistanceandcurrentLocation
🏁 Script executed:
#!/bin/bash
# Find where EcosystemBottomSheetContent is called (its call site)
rg -n "EcosystemBottomSheetContent" -g "*.kt" -A5 -B2
# Find where distanceStringToPlace is wired
rg -n "distanceStringToPlace\|distanceStringIfCurrentLocationExists" -g "*.kt" -B2 -A5
Length of output: 4176
🏁 Script executed:
#!/bin/bash
# Get the full EcosystemBottomSheetContent call in HomeScreen.kt
sed -n '330,380p' app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt
Length of output: 2662
Now I have the full picture. Here's the complete fix:
- Add a numeric
distanceIfCurrentLocationExiststoHomeViewModel - Add
distanceToPlace: (Double?, Double?) -> Double?parameter down the composable chain - Use it as the sort key in all four list functions
- Wire it up in
HomeScreen.ktand update the@Preview
Failed to handle agent chat message. Please try again.
| is ApiResponse.Success -> { | ||
| items(gymsApiResponse.data) { | ||
| items(gymsApiResponse.data | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) } |
There was a problem hiding this comment.
Sorting should ideally be done in the VM instead of directly in the UI
There was a problem hiding this comment.
Applies to all sorting in this file
| is ApiResponse.Success -> { | ||
| items(gymsApiResponse.data) { | ||
| items(gymsApiResponse.data | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) } |
There was a problem hiding this comment.
I think we can do sorting in one pass. Double check, but GPT is suggesting something like:
.sortedWith(
compareByDescending {
operatingHoursToString(it.operatingHours())
.contains("open", ignoreCase = true)
}.thenBy {
distanceToPlace(it.latitude, it.longitude)
}
)
| is ApiResponse.Success -> { | ||
| items(gymsApiResponse.data) { | ||
| items(gymsApiResponse.data | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) } |
There was a problem hiding this comment.
For Sorting logic, we might be able to reuse logic as a function some how maybe with a generic type
| items(gymsApiResponse.data | ||
| .sortedBy { distanceStringToPlace(it.latitude, it.longitude) } | ||
| .sortedByDescending { operatingHoursToString(it.operatingHours()) | ||
| .contains("open", ignoreCase = true) } |
There was a problem hiding this comment.
Is there a better way than checking for string in case the string changes? Maybe there can be some sort of function you can write.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt (2)
425-437: Naming inconsistency:numericalDistancetoPlaceshould benumericalDistanceToPlace.The function name uses inconsistent casing. In Kotlin camelCase convention, "to" should be capitalized as "To" since it's part of a compound word.
Proposed fix
- fun numericalDistancetoPlace(latitude: Double?, longitude: Double?): Double { + fun numericalDistanceToPlace(latitude: Double?, longitude: Double?): Double {Also update the call sites in
sortApiResponse(lines 452 and 454).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt` around lines 425 - 437, Rename the function numericalDistancetoPlace to numericalDistanceToPlace in HomeViewModel (update the function declaration) and update all call sites that use the old name—specifically the references inside sortApiResponse—so they call numericalDistanceToPlace; ensure access modifiers/signature remain unchanged and run a project-wide search to replace any other occurrences of numericalDistancetoPlace to avoid unresolved reference errors.
97-99: Consider using thelocationparameter or marking it with_for clarity.The
locationparameter captured from the combine is not used directly in the lambda body. Instead,numericalDistancetoPlacereadscurrentLocation.valueinternally. While this works correctly (the combine triggers re-evaluation when location changes), it creates confusing code where a parameter appears unused.Consider either:
- Passing
locationexplicitly tosortApiResponseornumericalDistancetoPlace- Using
_with a comment if the parameter is intentionally just for triggering recombinationOption 1: Pass location explicitly to sortApiResponse
- ) { printers, libraries, eateries, gyms, location -> + ) { printers, libraries, eateries, gyms, currentLoc -> StaticPlaces( printers = sortApiResponse( response = if (printers is ApiResponse.Success) { ApiResponse.Success(printers.data.filter { it.location.isNotBlank() }) } else { printers }, getLatitude = { it.latitude }, - getLongitude = { it.longitude } + getLongitude = { it.longitude }, + currentLocation = currentLoc ),This would require updating
sortApiResponseto accept and use the location parameter instead of reading the flow directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt` around lines 97 - 99, The lambda passed to combine captures a location parameter (from locationRepository.currentLocation) but never uses it—numericalDistancetoPlace reads currentLocation.value directly—so update the code to either (A) thread the combined location into the sorting call by modifying sortApiResponse/numericalDistancetoPlace to accept a Location argument and pass the location parameter from the lambda (references: gymRepository.gymFlow, locationRepository.currentLocation, sortApiResponse, numericalDistancetoPlace), or (B) if the parameter is only used to trigger recomputation, rename the parameter to _ (or _location) and add a short comment explaining it’s intentionally unused to avoid confusion; choose one approach and apply it consistently in HomeViewModel.kt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt`:
- Around line 425-437: Rename the function numericalDistancetoPlace to
numericalDistanceToPlace in HomeViewModel (update the function declaration) and
update all call sites that use the old name—specifically the references inside
sortApiResponse—so they call numericalDistanceToPlace; ensure access
modifiers/signature remain unchanged and run a project-wide search to replace
any other occurrences of numericalDistancetoPlace to avoid unresolved reference
errors.
- Around line 97-99: The lambda passed to combine captures a location parameter
(from locationRepository.currentLocation) but never uses
it—numericalDistancetoPlace reads currentLocation.value directly—so update the
code to either (A) thread the combined location into the sorting call by
modifying sortApiResponse/numericalDistancetoPlace to accept a Location argument
and pass the location parameter from the lambda (references:
gymRepository.gymFlow, locationRepository.currentLocation, sortApiResponse,
numericalDistancetoPlace), or (B) if the parameter is only used to trigger
recomputation, rename the parameter to _ (or _location) and add a short comment
explaining it’s intentionally unused to avoid confusion; choose one approach and
apply it consistently in HomeViewModel.kt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97144dbd-b48e-409a-a7fa-451d91d864cd
📒 Files selected for processing (2)
app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.ktapp/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt (2)
442-461: Minor formatting: missing spaces after commas.Lines 452 and 454 are missing spaces after commas in the function calls.
✨ Proposed fix
if (getIsOpen != null) { compareByDescending<T> { getIsOpen(it) } - .thenBy { numericalDistanceToPlace(getLatitude(it),getLongitude(it)) } + .thenBy { numericalDistanceToPlace(getLatitude(it), getLongitude(it)) } } else { - compareBy { numericalDistanceToPlace(getLatitude(it),getLongitude(it)) } + compareBy { numericalDistanceToPlace(getLatitude(it), getLongitude(it)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt` around lines 442 - 461, The formatting issue in sortApiResponse is missing spaces after commas in calls to numericalDistanceToPlace; update the calls inside sortedWith so numericalDistanceToPlace(getLatitude(it), getLongitude(it)) (and similarly wherever numericalDistanceToPlace is invoked) include a space after the comma, keeping parameter names getLatitude, getLongitude and optional getIsOpen unchanged to fix spacing while preserving functionality.
97-126: Unusedlocationparameter in combine lambda.The
locationparameter received fromcombineis never used. Instead,numericalDistanceToPlacereadscurrentLocation.valuedirectly. While this works because they reference the same source, it's inconsistent and could cause subtle timing issues if the location changes between when combine emits and when sorting reads the value.Consider passing the location to
sortApiResponseor usinglocationdirectly within the lambda for consistency.♻️ Proposed fix: Pass location to sortApiResponse
- private fun <T> sortApiResponse( + private fun <T> sortApiResponse( response: ApiResponse<List<T>>, getLatitude: (T) -> Double?, getLongitude: (T) -> Double?, - getIsOpen: ((T) -> Boolean)? = null + getIsOpen: ((T) -> Boolean)? = null, + currentLoc: LatLng? = null ): ApiResponse<List<T>> { if (response is ApiResponse.Success) { val sortedData = response.data.sortedWith( if (getIsOpen != null) { - compareByDescending<T> { getIsOpen(it) } - .thenBy { numericalDistanceToPlace(getLatitude(it),getLongitude(it)) } + compareByDescending<T> { getIsOpen(it) } + .thenBy { numericalDistanceToPlace(getLatitude(it), getLongitude(it), currentLoc) } } else { - compareBy { numericalDistanceToPlace(getLatitude(it),getLongitude(it)) } + compareBy { numericalDistanceToPlace(getLatitude(it), getLongitude(it), currentLoc) } } ) return ApiResponse.Success(sortedData) } else { return response } }Then update call sites to pass
location:printers = sortApiResponse( response = ..., getLatitude = { it.latitude }, getLongitude = { it.longitude }, currentLoc = location ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt` around lines 97 - 126, The combine lambda currently receives a location parameter that is unused while sortApiResponse and numericalDistanceToPlace read currentLocation.value directly; update sortApiResponse calls inside the lambda (for printers, libraries, eateries, gyms) to accept and use the emitted location (e.g., add a currentLoc parameter to sortApiResponse) and pass the lambda's location through, and update numericalDistanceToPlace to accept/use that passed currentLoc instead of reading currentLocation.value directly so the sorting uses the exact combined emission; reference sortApiResponse, numericalDistanceToPlace, and the combine lambda to locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt`:
- Around line 442-461: The formatting issue in sortApiResponse is missing spaces
after commas in calls to numericalDistanceToPlace; update the calls inside
sortedWith so numericalDistanceToPlace(getLatitude(it), getLongitude(it)) (and
similarly wherever numericalDistanceToPlace is invoked) include a space after
the comma, keeping parameter names getLatitude, getLongitude and optional
getIsOpen unchanged to fix spacing while preserving functionality.
- Around line 97-126: The combine lambda currently receives a location parameter
that is unused while sortApiResponse and numericalDistanceToPlace read
currentLocation.value directly; update sortApiResponse calls inside the lambda
(for printers, libraries, eateries, gyms) to accept and use the emitted location
(e.g., add a currentLoc parameter to sortApiResponse) and pass the lambda's
location through, and update numericalDistanceToPlace to accept/use that passed
currentLoc instead of reading currentLocation.value directly so the sorting uses
the exact combined emission; reference sortApiResponse,
numericalDistanceToPlace, and the combine lambda to locate the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7deb73d1-1af3-4afe-b426-d208606c2829
📒 Files selected for processing (1)
app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt
2 participants
Overview
Sorted Bottom Sheet Content
Changes Made
Test Coverage
Next Steps (delete if not applicable)
Screenshots (delete if not applicable)
Sorted Gyms
[Screen_recording_20260318_182019.webm](https://github.com/user-attachments/assets/3a40370e-8269-4753-82b6-7e97a3bf100b)Sorted Eateries
[Screen_recording_20260318_182726.webm](https://github.com/user-attachments/assets/50ed7048-106b-4ff6-8589-456609698389)Sorted Libraries
[Screen_recording_20260318_182827.webm](https://github.com/user-attachments/assets/0ee1f22f-c83a-4851-bd6d-6d68357a9b0a)Sorted Printers
[Screen_recording_20260318_182854.webm](https://github.com/user-attachments/assets/31dabc82-0ee9-48d4-b5da-264f6c24ab71)Summary by CodeRabbit