Can someone review this snippet of code to tell me...

# compose

[Colton Idle]

Can someone review this snippet of code to tell me whether or not a remember{} is needed? Code in thread

[Colton Idle]

I have

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

val colorsForStatus = colorsForStatus(status)
Box(
    modifier =
        Modifier.fillMaxWidth(1f)
          .background(brush = Brush.verticalGradient(colorsForStatus)))

and the function itself (which is not a composable function)

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))
                      }
  }
}

[Colton Idle]

Is my code fine like this, or should I be wrapping

val colorsForStatus = colorsForStatus(status)

in a remember {}?

[natario1]

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() … ).

[Adam Powell]

+1 to making a static lookup table for these

[Colton Idle]

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?

[natario1]

Making a Map<Status, Brush> instead of a function

[Colton Idle]

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?

val myLookuptable = mapOf(Pair(Status.ABC, listOf(a bunch of hex values)))

if so, then I would just use it like this right?

myLookuptable.getOrDefault(status, listOf(default list of hex values))

[Colton Idle]

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.

[Colton Idle]

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!

[Colton Idle]

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"?

[Adam Powell]

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 🙃

[Adam Powell]

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

[Colton Idle]

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?

[Adam Powell]

yep

[Colton Idle]

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

[Adam Powell]

it might be useful to know that `remember {}`'s original api name was `memo {}`: https://en.wikipedia.org/wiki/Memoization

[james]

☝️ 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 😁

[Colton Idle]

Oh freakin awesome. So happy I asked my question. The space tradeoff though in this case is in the slot table right?

[Adam Powell]

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. 😛