Is it considered more idiomatic to prefer combinin...

# codingconventions

[Jesse Hill]

Is it considered more idiomatic to prefer combining two lists using + instead of creating a MutableList, adding all the values and then changing the MutableList to a List with toList? Code in 🧵 .

[Jesse Hill]

Example 1:

combine(
    repository.getAllTicketsOfType(),
    repository.getAllTicketsOfOtherType()
) { firstTickets, secondTickets ->
   val list: MutableList<TicketDetails> = mutableListOf()

   list.addAll(firstTickets)
   list.addAll(secondTickets)

   list.toList()
}.collect { 
    it.forEach { ...do something with tickets } 
}

Example 2:

combine(
    repository.getAllTicketsOfType(),
    repository.getAllTicketsOfOtherType()
) { firstTickets, secondTickets ->
    firstTickets + secondTickets
}.collect { 
    it.forEach { ...do something with tickets } 
}

[Orhan Tozan]

2 is more readable

[Luke]

I would say 2️⃣, and if the logic complexifies, then use buildList

[Paul Griffith]

More broadly, in this specific example, if you need to iterate everything from one list and then everything from the other list, I'd use a sequence block (I don't think you need a flow here at all)

sequence {
    yieldAll(repository.getAllTicketsOfType())
    yieldAll(repository.getAllTicketsOfOtherType())
}.forEach { ticket ->
 TODO("Something with tickets") 
}

[Jesse Hill]

Thanks for the suggestions!

@Paul Griffith The reason I’m using a flow here is because the repository returns Flow<List<TicketType>> so I think I need combine to put them into a single List when the flow is collected. I tried out the sequence as you suggested but yieldAll expects Iterator, Iterable, or Sequence so doing repository.getAllTicketsOfType() didn’t work because it returns a flow. That is a good reminder about sequence though, I keep forgetting about it 😂

[Paul Griffith]

Ah, it looks like there's a top level flow builder that might work (very similar to sequence :

flow {
    emitAll(repository.getAllTicketsOfType())
    emitAll(repository.getAllTicketsOfOtherType())
}.collect { ticket -> 
    TODO("Do something with each ticket")
}

[Michael Böiers]

You can also do sequenceOf(list1, list2).flatten().forEach { … } (for the initial example of processing multiple lists of course, this doesn’t work for flows)

[Klitos Kyriacou]

By the way, a small note about overhead and performance. In your first example, list.toList() returns a copy of the list, which is unnecessary - you could just return list as a List (which MutableList extends) so that there is no copying.