Optimizing a for loop to add items to a map

user_70942 0
Viewed 385
Kotlin 1.5.42

I have the following data class and I need to filter out level 2 and level 3 values. Then I use a for..loop to loop through the level2 and check that the level3 has a parentId that matches the level2 value Which is added to a list. When that level3 loop has finished I add to map with the key being the level2 object and the value being the list of level3.

i.e. Map<TopsProductFilterItem, List<TopsProductFilterItem>>

I was looking for a better solution that is more concise using kotlin and looking if associateBy, or assoicateWith would help.

data class TopsProductFilterItem(
    val value: String = "",
    val catalogSearchCustomAttribute: CatalogSearchCustomAttribute = CatalogSearchCustomAttribute(),
)

And the following data class that has the levels

data class CatalogSearchCustomAttribute(
        val level: Int = 0,
        val parentId: Int = 0)

As there can be many levels assigned to his class I am only interested in level 2 and level 3. So I have filtered them out as below.

private fun createMapOfLevelCategories(listOfTopsProductFilterItem: List<TopsProductFilterItem>) {
    val listOfLevel2 = listOfTopsProductFilterItem.filter { topsProductFilterItem ->
        topsProductFilterItem.catalogSearchCustomAttribute.level == 2
    }

    val listOfLevel3 = listOfTopsProductFilterItem.filter { topsProductFilterItem ->
        topsProductFilterItem.catalogSearchCustomAttribute.level == 3
    }

    val mapOfCategoryLevel2 = mutableMapOf<TopsProductFilterItem, List<TopsProductFilterItem>>()

    listOfLevel2.forEach { categoryLevel2 ->
        /* Find the parent id in the level 3 and from the level 2 value */
        val listOfCategoryLevel3 = mutableListOf<TopsProductFilterItem>()
        listOfLevel3.forEach { categoryLevel3 ->
            if(categoryLevel2.value.toInt() == categoryLevel3.catalogSearchCustomAttribute.parentId) {
                /* found a matching parent ID and value */
                listOfCategoryLevel3.add(categoryLevel3)
            }
        }
        mapOfCategoryLevel2.put(categoryLevel2, listOfCategoryLevel3)
    }
}
kotlin for-loop optimization
4 Answers

You can do listOfLevel2.associateWith { ... }. associatedWith creates map with the keys being the same as the iterable you called it on, but allows you to specify how you want each key's associated value to be transformed. In this case, we want the value to be all the items in the level 3 list that have a parentId equal to the key's value.toInt().

listOfLevel2.associateWith { level2 ->
    listOfLevel3.filter { level3 ->
        level2.value.toInt() == level3.catalogSearchCustomAttribute.parentId
    }
}

Even better, you can do a groupBy on the level 3 parent Ids first, then you don't have to loop over the the listOfLevel3 over and over again in associatedWith:

listOfLevel3.groupBy { it.catalogSearchCustomAttribute.parentId }.let { parentIdGroups ->
    listOfLevel2.associateWith { level2 ->
        parentIdGroups[level2.value.toInt()] ?: emptyList()
    }
}
user_51335850

It may be a matter of taste regarding code readability, but I would do it like this:

private fun createMapOfLevelCategories(listOfTopsProductFilterItem: List<TopsProductFilterItem>): Map<TopsProductFilterItem, List<TopsProductFilterItem>> {
    // create map: parentId -> (parent, children)
    val resultById = listOfTopsProductFilterItem
        .filter { it.catalogSearchCustomAttribute.level == 2 }
        .associate {
            it.value.toInt() to Pair(it, mutableListOf<TopsProductFilterItem>())
        }

    // associate children to parents
    listOfTopsProductFilterItem
        .filter { it.catalogSearchCustomAttribute.level == 3 }
        .forEach {
            resultById.getValue(it.catalogSearchCustomAttribute.parentId).second += it
        }

    return resultById.values.toMap()
}

I wouldn't say it is trivial to read and understand, but at least for me it is cleaner than your implementation. It should be also a little more performant, because we iterate over level3 items only once, not once per level2 item.

user_4488750

There is a balance between concise and readable that is subjective. This works for me:

fun mapOfLevel2Categories(itemList: List<TopsProductFilterItem>): Map<TopsProductFilterItem, List<TopsProductFilterItem>> {

    fun List<TopsProductFilterItem>.withParent(item: TopsProductFilterItem) =
        this.filter { it.catalogSearchCustomAttribute.parentId == item.value.toInt() }

    val (level2items, level3items) = itemList
        .filter { it.catalogSearchCustomAttribute.level in (2..3) }
        .partition { it.catalogSearchCustomAttribute.level == 2 }

    return level2items.associateWith { level3items.withParent(it) }
}

By the way, having the string TopsProductFilterItem.value as the id is odd. I am guessing that this is due to simplifying the actual implementation for SO.

user_2391010

This can be achieved in one statement-chain using associateWith.

fun createMapOfLevelCategories(
  listOfTopsProductFilterItem: List<TopsProductFilterItem>,
): Map<TopsProductFilterItem, List<TopsProductFilterItem>> {

  val upperLevel = 2
  val lowerLevel = 3

  return listOfTopsProductFilterItem
    // find all upper-level items
    .filter { it.catalogSearchCustomAttribute.level == upperLevel }
    // find all children for the upper-level item
    .associateWith { upperLevelItem ->
      // search the item list for children of the upperLevelItem...
      listOfTopsProductFilterItem
        // find all lower-level items
        .filter { it.catalogSearchCustomAttribute.level == lowerLevel }
        // find all lower-level items with a parent in the upper-level 
        .filter { it.catalogSearchCustomAttribute.parentId == upperLevelItem.value.toInt() }
    }
}

If searching filtering through the list repetitively is expensive, then use groupBy to create a Map. Then all children for a given parentId can be easily fetched.

fun createMapOfLevelCategories(
  listOfTopsProductFilterItem: List<TopsProductFilterItem>,
): Map<TopsProductFilterItem, List<TopsProductFilterItem>> {

  val upperLevel = 2
  val lowerLevel = 3

  // group all lower-level Items by parentId
  val mapParentIdToChildItems: Map<Int, List<TopsProductFilterItem>> =
    listOfTopsProductFilterItem
      .filter { it.catalogSearchCustomAttribute.level == lowerLevel }
      .groupBy { it.catalogSearchCustomAttribute.parentId }

  return listOfTopsProductFilterItem
    // find all upper-level items
    .filter { it.catalogSearchCustomAttribute.level == upperLevel }
    // find all children for the upper-level item
    .associateWith { upperLevelItem ->
      mapParentIdToChildItems[upperLevelItem.value.toInt()] ?: listOf()
    }
}
Assumptions

I've made two assumptions,

  • TopsProductFilterItem.value is a unique identifier,
  • The relationship is one parent has many children, not many-to-many.
user_41614710
Related