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

Colton Idle

05/11/2022, 2:24 AM
Can someone review this snippet of code to tell me whether or not a
remember{}
is needed? Code in thread
I have
Copy code
Box(
    modifier =
        Modifier.fillMaxWidth(1f)
            .background(
                brush =
                    when (status) {
                      Status.ABC,
                      Status.DEF -> {
                        Brush.verticalGradient(listOf(bunch of hex values))
                      }
                      <http://Status.XYZ|Status.XYZ> -> {
                        Brush.verticalGradient(listOf(bunch of hex values))
                      }
...
I'd like to extract the "when" statement into just a function so it doesn't have to pollute my composable. So I basically did this
Copy code
val colorsForStatus = colorsForStatus(status)
Box(
    modifier =
        Modifier.fillMaxWidth(1f)
          .background(brush = Brush.verticalGradient(colorsForStatus)))
and the function itself (which is not a composable function)
Copy code
fun colorsForStatus(status: String): List<Color> {
  return when (status) {
                      Status.ABC,
                      Status.DEF -> {
                        Brush.verticalGradient(listOf(bunch of hex values))
                      }
                      <http://Status.XYZ|Status.XYZ> -> {
                        Brush.verticalGradient(listOf(bunch of hex values))
                      }
  }
}
Is my code fine like this, or should I be wrapping
Copy code
val colorsForStatus = colorsForStatus(status)
in a remember {}?
n

natario1

05/11/2022, 2:36 AM
It’s fine. You’d use remember(state) to avoid heavy computations, which is not happening in your case unless your lists are huge. Personally if this stuff is really static, I might use a top level mapOf(ABC to verticalGradient() … ).
a

Adam Powell

05/11/2022, 2:47 AM
+1 to making a static lookup table for these
c

Colton Idle

05/11/2022, 2:48 AM
lost me at "I might use a top level mapOf(ABC to verticalGradient() … )." and "static lookup table" Do you just mean moving my function outside of my class and just having it be a "global" function?
n

natario1

05/11/2022, 2:57 AM
Making a Map<Status, Brush> instead of a function
👍 1
c

Colton Idle

05/11/2022, 2:58 AM
Edit: Your message came in right as I hit send on this. thanks Oh. I think I misunderstood. You all just mean something like this?
Copy code
val myLookuptable = mapOf(Pair(Status.ABC, listOf(a bunch of hex values)))
if so, then I would just use it like this right?
Copy code
myLookuptable.getOrDefault(status, listOf(default list of hex values))
So it's a little more verbose at the call site since I still need to handle the default case, which my function would've been able to handle itself.
I'm sure im in the realm of microoptimzations now, but its funny because i actually didn't think of just having a map. so thank you both for that idea!
also. reading the compose/remember docs it doesn't necessarily call out the fact that it should only be used on "heavy operations". Does anyone have a source that I can link in my PR explaining why I didn't surround in remember besides just saying "Mattia and Adam P said its fine"?
a

Adam Powell

05/11/2022, 3:14 AM
or even just
fun brushFor(status: Status): Brush = myLookupTable[status] ?: defaultBrush
- the point is you only have one allocated brush per status for the life of the process and you reuse it. You don't need to remember a global 🙃
when you
remember
something you're trading composition slot table space for computation time (and that computation time might come in the form of gc pressure if you're allocating big/complex object graphs) and for keyed remembers you're spending time comparing keys for equality to see if you need to do work
3
c

Colton Idle

05/11/2022, 3:18 AM
Interesting. Okay. Now that you say it again it makes sense. For each time I would currently call colorsForStatus() it gets a new color list. And since I actually do this in a lazyColumn then that's actually a lot. Going with a map, my instances of color lists are all created at once, and i just grab em as I need em. Is that basically right @Adam Powell?
a

Adam Powell

05/11/2022, 3:19 AM
yep
1
❤️ 1
c

Colton Idle

05/11/2022, 3:21 AM
For the decision on whether or not to use remember, my team has mostly almost always agreed on the mantra "hey you probably should wrap this in a remember or else it'll run on every composition" which has def swayed us towards using remember a lot. But looking through the docs there isn't anything that calls this out as not being something to use for "light" function calls like my original function. Anything I can point my team towards that you might have handy as some reading material or should I send em this thread? lol
a

Adam Powell

05/11/2022, 3:39 AM
it might be useful to know that `remember {}`'s original api name was `memo {}`: https://en.wikipedia.org/wiki/Memoization
🤯 2
j

james

05/11/2022, 4:10 AM
☝️ this makes total sense, but my reaction is still 🤯 when I was first starting to learn Compose,
remember
took me some time to get my head around, but that little bit of trivia would’ve helped it “click” more quickly for me petition to add that “fun fact” to the official docs 😁
😄 4
c

Colton Idle

05/11/2022, 1:27 PM
Oh freakin awesome. So happy I asked my question. The space tradeoff though in this case is in the slot table right?
a

Adam Powell

05/11/2022, 2:24 PM
or more generally, in the composition, yes, with the slot table as an implementation detail that may or may not change. But people like to nod sagely whenever someone says, "slot table" so sure. 😛
🎉 1
3 Views