[Feature request] Change the confusing markerState parameter position to a clear name.

[JSpiner]

MarkerState has rememberMarkerState for use in compose.

@Composable
public fun rememberMarkerState(
    key: String? = null,
    position: LatLng = LatLng(0.0, 0.0)
): MarkerState = rememberSaveable(key = key, saver = MarkerState.Saver) {
    MarkerState(position)
}

// wrong usage example
@Composable
public fun MyGoogleMapScreen(stationPosition: LatLng) {
    val myMarkerState = rembmerMarkerState(position = stationPosition)
}

But looking at the example above, it's easy to get confused.
This is because there is a risk of misunderstanding that the value entered as a parameter to remember (position in this case) acts as a key that is automatically reflected when the value changes.
I also had a hard time because the position wasn't updated.
To avoid this misunderstanding, compose foundation libraries add an inital prefix to rememberXXX functions.

// compose library example
// rememberScrollState from `androidx.compose.foundation`
@Composable
fun rememberScrollState(initial: Int = 0): ScrollState {
    return rememberSaveable(saver = ScrollState.Saver) {
        ScrollState(initial = initial)
    }
}

// rememberLazyListState from `androidx.compose.foundation.lazy`
@Composable
fun rememberLazyListState(
    initialFirstVisibleItemIndex: Int = 0,
    initialFirstVisibleItemScrollOffset: Int = 0
): LazyListState {
    return rememberSaveable(saver = LazyListState.Saver) {
        LazyListState(
            initialFirstVisibleItemIndex,
            initialFirstVisibleItemScrollOffset
        )
    }
}

// rememberLazyGridState from `androidx.compose.foundation.lazy.grid`
@Composable
fun rememberLazyGridState(
    initialFirstVisibleItemIndex: Int = 0,
    initialFirstVisibleItemScrollOffset: Int = 0
): LazyGridState {
    return rememberSaveable(saver = LazyGridState.Saver) {
        LazyGridState(
            initialFirstVisibleItemIndex,
            initialFirstVisibleItemScrollOffset
        )
    }
}

So my suggestion is to change the name from position to initialPosition to reduce confusion.
It seems like a simple fix, I'll create a PR. Please review it.
Thanks!

[bubenheimer]

Personally I'd be in favor of eliminating rememberMarkerState entirely. I find it misleading and an anti-pattern:

https://dev.to/bubenheimer/effective-map-composables-non-draggable-markers-2b2#:~:text=Be%20aware%20that,recommend%20ignoring%20it.

https://dev.to/bubenheimer/effective-map-composables-draggable-markers-3bea#:~:text=This%20behavior%20is,with%20a%20model.

[dkhawk]

That is an interesting proposition. Admittedly, I have had issues with the way it works as well. Might be worth investigating as a possibility.

…

On Wed, Oct 16, 2024, 15:14 Uli Bubenheimer ***@***.***> wrote: Personally I'd be in favor of eliminating rememberMarkerState entirely. I find it misleading and an anti-pattern: https://dev.to/bubenheimer/effective-map-composables-non-draggable-markers-2b2#:~:text=Be%20aware%20that,recommend%20ignoring%20it . https://dev.to/bubenheimer/effective-map-composables-draggable-markers-3bea#:~:text=This%20behavior%20is,with%20a%20model . — Reply to this email directly, view it on GitHub <#637 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2GLJ4EXEB2OMQF6IVYXLZ3X73ZAVCNFSM6AAAAABQATZMWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJVHAZDAOJZGM> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[JSpiner]

@dkhawk

Yes, it can be confusing. Not only me, but my coworkers are also confused.

android-maps-compose/app/src/main/java/com/google/maps/android/compose/markerexamples/updatingnodragmarkerwithdatamodel/UpdatingNoDragMarkerWithDataModelActivity.kt

Line 150 in f7f4854

fun rememberUpdatedMarkerState(position: LatLng): MarkerState =

@Composable
fun rememberUpdatedMarkerState(position: LatLng): MarkerState =
    // This pattern is equivalent to what rememberUpdatedState() does:
    // rememberUpdatedState() uses MutableState, we use MarkerState.
    // This is more efficient than updating position in an effect,
    // as we avoid an additional recomposition.
    remember { MarkerState(position = position) }.also {
        it.position = position
    }

The example code included in this repository also creates and uses rememberUpdatedMarkerState to solve this problem.
I also solved the problem in a similar way.
It seems okay to provide this function, but I think the first thing to do is to distinguish between initPosition and position so that there is no confusion.

#638
Can you review my PR and leave comments?