-
Notifications
You must be signed in to change notification settings - Fork 2
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
Search by place. String provider. Detekt. #42
Search by place. String provider. Detekt. #42
Conversation
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.
I think we can (or should) generally prefer idiomatic code wherever possible.
suspend fun getSearchResults(query: String): List<SearchResult> = withContext(Dispatchers.IO) { | ||
if (query.isEmpty()) return@withContext emptyList<SearchResult>() | ||
|
||
val stops = getStops(query) |
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.
val stops = getStops(query) | |
getStops(query).takeIf { it.isNotEmpty() }?.let { /* Add header and append to search results */ } |
This could be made more idiomatic by using a takeIf
followed by a let
block where the header and results can be added to the output list.
.executeCall() | ||
.body() | ||
?.features() | ||
?.map { SearchResult.PlaceItem(it.placeName().orEmpty(), it.text().orEmpty()) } |
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.
?.map { SearchResult.PlaceItem(it.placeName().orEmpty(), it.text().orEmpty()) } | |
?.map { feature -> SearchResult.PlaceItem(feature.placeName().orEmpty(), feature.text().orEmpty()) } |
Add explicit lambda parameter name here
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.
Any particular reason for this? It's not overshadowed or anything. Are we avoiding the it
implicit parameter altogether?
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.
I think it's more readable this way. it explicitly states what's being dealt with in the closure since it's a bit too confusing to infer at the bottom of a long chain of function calls.
Issues
Closes #28
Summary
Use
./gradlew detekt
to check for lint errorsTodo #43
Screenshots