https://kotlinlang.org logo
#compose
Title
# compose
r

Roar Gronmo

12/06/2019, 11:25 AM
Compose, multiple parameters which could be lambdas: My suggestion is to have a
@Lambda
annotation in front of the function which should intendly be the lambda function. i.e.:
Copy code
private fun BottomBar(
    post: Post, 
    @Lambda onUnimplementedAction: () -> Unit){
...
}
If it isn't already suggested before. RG
b

bmo

12/06/2019, 11:31 AM
I fail to see what is the added benefit of this annotation. Could you elaborate?
1
r

Roar Gronmo

12/06/2019, 12:09 PM

https://youtu.be/i9RJpMOsKas

Listen to this from approx 43800, there is this: "What should be the lambda..."
a

Andrew Kelly

12/06/2019, 12:17 PM
My thought on that issue, where if you add a
Button(text = "Hello") { Text("world")}
is that the compiler should throw a warning, that a @Composible shouldn’t be allowed as a click handler…there is enough info already to warn the user that the lambda is not working as they expect it to be.
r

Roar Gronmo

12/06/2019, 12:27 PM
Cool if you could define an array of lambdas... not sure how it should be implemented in Kotlin (or is it already ?).
Copy code
private fun BottomBarAction(
    @DrawableRes id: Int,
    onClick: ()[1] -> Unit,
    onLongClick: ()[2] -> Unit
)[1]{
   //HandleClick    
}[2]{
  //HandleLongClick
}
Maybe bad suggestion 😋
b

bmo

12/06/2019, 12:45 PM
Thank you, the video made it clearer 🙂 I would say that if that would be a solution (I'm not a big fan of it but that's another question) it should be something in Kotlin directly and not allow Compose to change the language itself. Same with the array, I would say. Regarding the array, the proper usage would be :
Copy code
BottomBarAction(
    id = someId,
    onClick = { ... },
    onLongClick = {... }
)
The trailing lambda is a nice touch but it can be tricky indeed as she points it out. Right now, their solution is good as it avoid overlaps of method signatures but it forces them to be very careful not to break it.
r

Roar Gronmo

12/06/2019, 12:51 PM
Of course @bmo it is more wise to let the on*** parameters to be the entryponts to some actions. Kotlin is fresh (1.3.61) , so there are situations like this that takes a beatiful language a little bit further...
b

bmo

12/06/2019, 12:55 PM
Sorry, I didn't mean to offend you. I just meant that it should be handled in the language rather than have Compose modify the language itself. So maybe having an annotation which prevent trailing lambdas should be proposed to kotlin directly
r

Roar Gronmo

12/06/2019, 1:03 PM
Hope they (Kotlin/JetBrain) read this post... ...so we may have a point @bmo that a
@Lambda
may be a solution... (Psst: I'm not offended, I'm to old for that). 🧓
Hmm... in the other end, we can find ourselves choosing between:
Copy code
Column {
    TopAppBar(
        title = { Text(text = "JetNews") },
        navigationIcon = {
            VectorImageButton(id = R.drawable.ic_jetnews_logo) {
                openDrawer()
            }
        }
    )
}
or
Copy code
Column {
    TopAppBar(
        title = { Text(text = "JetNews") },
        navigationIcon = {
            VectorImageButton(
                id = R.drawable.ic_jetnews_logo, 
                onClick = {
                    openDrawer()
                }
            )
        }
    )
}
I must say the latter one is to prefer, because you understand what the actually action does (is it
onClick
,
onLongClick
or something else...?). But not using the lambda method, will we miss some functionality, or are all the options/benefits there ?
Found another example of "ambiguity" when working with this problem, the definition of a TabRow is like this:
Copy code
@Composable
public fun <T> TabRow(
    items: List<T>,
    selectedIndex: Int,
    scrollable: Boolean,
    indicatorContainer: @Composable()
(List<TabRow.TabPosition>) → Unit,
    tab: @Composable()
(Int, T) → Unit
): Unit
At the definition now (as I understand), the lambda points to the latest added parameter,
tab: @Composable() (Int, T)->Unit,
but it could in theory be the
indicatorContainer: @Composable() (List<TabRow.TabPosition>) -> Unit
. So to make that happen, we need to override and switch theese two parameters ? I rather stick to using parameter reference here... BTW: If there was this
@Lambda
annotation defining which one was the defined lambda, when an ambiguity existed, it would be clear for everybody, and you could define it anywhere in the sequence of lambda likes, but only one (at this time, maybe named ones later
@Lambda(MyLambdaName)
. How those should be referenced in the code structure, I'm not sure... )
l

Leland Richardson [G]

12/11/2019, 5:58 PM
Running up on this thread a bit late, but I don’t think there is anything at the kotlin level that needs to be added here. As far as kotlin is concerned, there is no ambiguity here. The spec for what lambda gets the trailing lambda syntax is reasonably clear. The question really comes down to API design, and whether or not we should make our APIs consistent. When you are mixing “callback/event parameters” with “composable parameters” in overloads, it becomes harder to know what the code is doing.
r

Roar Gronmo

12/11/2019, 6:54 PM
Good point @Leland Richardson [G], I surely prefer consistent APIs. My opinion (now, when listening to you) is that the lambda should be the main "subject" of the function, i.e. for Button should onClick be the lambda.
l

Leland Richardson [G]

12/11/2019, 7:35 PM
so the question is what do we do when we want to have the following two overloads of Button?
Copy code
Button(text="Hello", onClick={ ... })
and
Copy code
Button(onClick={ ... }, content={ Text("Hello") })
with named parameters it’s clear
but if named parameters are ommitted, it’s odd
because
Copy code
Button { println("Hello world") }
and
Copy code
Button("Click me") { println("hello world") }
(the former is an error in this particular case, to be fair)
but the problem is that the semantic of the lambda is different in the overloads of the same function, which i think we should probably try and never do
so you could say that for button, handler is most important
and then the correct form of the former is
Copy code
Button({ Text("Click me") }) { println("hello world") }
we decided the opposite… that it was more important to have the consistency of a trailing lambda on the composable implying hierarchy
but i can see the argument for either
r

Roar Gronmo

12/11/2019, 7:48 PM
Oh... think I'll stick to named parameters ... 😖 ...for a while... ...(to be discussed somewhere)...
😃😃
On the other hand, I rather don't like that the order of something (here parameters) should define what that something (parameter) should do. It fights against kotlins flexibility or conciseness. Instead of an annotation (@Lambda) there should rather be a "lambda" prefix for that perticular lambda-possible parameter, which you could define when you called that function... (I'll explain later).
l

Leland Richardson [G]

12/11/2019, 8:03 PM
this ship has already sailed though…. trailing lambda syntax applies to the last parameter only. this means that it will affect API design, and thus, parameter ordering
and fwiw i don’t think API design should ignore parameter order anyway.
r

Roar Gronmo

12/11/2019, 8:08 PM
Are there been any strong opinions on this particular situation (order directed lambdas) from JetBrains??
Here's my suggestion for lambda definition (and I know, it isn't good)...:
Copy code
...
MaterialTheme {
   ModalDrawerLayout(
       drawerState = drawerState,
       onStateChange = onDrawerStateChange,
       gesturesEnabled = drawerState == Opened, 
       lambda drawerContent,  //<-- lambda prefix
       bodyContent = {
           AppContent(
                   openDrawer = {
                       onDrawerStateChanged(Opened)
                   }
           )
       } 
   ){       //lambda is drawerContent
      AppDrawer(
              closeDrawer = {
                  onDrawerStateChange(Closed)
              }
      )
   }
}
(for you who reads this, it will not work in kotlin (at the moment 😁) ) RG
In the API definitions it cold also be used:
Copy code
@Composable
fun ModalDrawerLayout(
    drawerState: DrawerState,
    onStateChange: (DrawerState) -> Unit,
    gesturesEnabled: Boolean = true,
    drawerContent: @Composable() () -> Unit,
    lambda bodyContent: @Composable() () -> Unit ) //<- it clearly shows which parameter is the defined lambda (and it is simply overridable if it should be used otherwise), as shown in previous example.
(for you who reads this, it will not work in kotlin (at the moment 😁) ) RG
...or maybe better postfix... ("java_me" struck through)
Copy code
@Composable
fun ModalDrawerLayout(
    drawerState: DrawerState,
    onStateChange: (DrawerState) -> Unit,
    gesturesEnabled: Boolean = true,
    drawerContent: @Composable() () -> Unit,
    bodyContent: @Composable() () -> lambda Unit ) //<- it clearly shows which parameter is the defined lambda (and it is simply overridable if it should be used otherwise), as shown in previous example.
(for you who reads this, it will not work in kotlin (at the moment 😁) ) RG
... or even more postfix ...
Copy code
@Composable
fun ModalDrawerLayout(
    drawerState: DrawerState,
    onStateChange: (DrawerState) -> Unit,
    gesturesEnabled: Boolean = true,
    drawerContent: @Composable() () -> Unit,
    bodyContent: @Composable() () -> Unit lambda ) //<- it clearly shows which parameter is the defined lambda (and it is simply overridable if it should be used otherwise), as shown in previous example.
(for you who reads this, it will not work in kotlin (at the moment 😁) ) RG
And it could be "grayed" when it was written last, since this is the default behavior. Otherwise the keyword, overrides the last one as this:
Copy code
@Composable
fun ModalDrawerLayout(
    drawerState: DrawerState,
    onStateChange: (DrawerState) -> Unit,
    gesturesEnabled: Boolean = true,
    drawerContent: @Composable() () -> Unit lambda, //<-
    bodyContent: @Composable() () -> Unit)
// Here drawerContent is lambda.
(for you who reads this, it will not work in kotlin (at the moment 😁) ) RG
49 Views