Hi all, I would like to improve this by removing mutable vars here, but I’m kind of struggling with ...

# random

[Alex]

Hi all, I would like to improve this by removing mutable vars here, but I’m kind of struggling with finding a way to do so. Any ideas?

[Alex]

The raw string is something like a121_c312_t1231_dasdasdaada and can may contain unnecessary “keys” like a8888 at the end so it’s important for the first keys to have precedence

[Fleshgrinder]

No need to overcomplicate things. 😉

val affiliateId: String?

init {
    var affiliateId: String? = null
    // snip
    this.affiliateId = affiliateId
}

[Alex]

damn it it was so obvious that now I feel stupid. Thank you! 😄

[Fleshgrinder]

You can also get rid of the init block if you desire:

class C(val a: String?, val b: String?, val c: String?) {
  companion object {
    operator fun invoke(a: String?): C {
      var b: String? = null
      var c: String? = null
      // snip
      return C(a, b, c)
    }
  }
}

This is useful for unit testing (object mothers), because now you can freely assign values to b and c without going through the init logic. Obviously the constructor can be internal if you want to keep this possibly away from others.

[Alex]

hm, I’m not invoking the class manually though, Jackson deserializes a JSON string and instantiates the object, I’m not sure that would work

[Fleshgrinder]

companion object {
  @JvmStatic
  @JsonCreator
  operator fun invoke() {}
}

[Alex]

I finally understood what you meant. init { } actually won’t work and companion object is the way to go. 😄

[Ivan Pavlov]

@Alex
I have some suggestions which could probably help you to simplify your logic.
1. Inside filter you check both isNotEmpty (length > 0) and substring(1).isNotEmpty() (length > 1) which can be probably simplified with it.length
2. You reverse list and then iterate with forEach. In this forEach if you find prefix, you set corresponding variable. If you have multiple items with the same prefix you will override property. This means that the latest (in reversed list) set wins. And latest in reversed is the first in original.
If you put things together you can end up with something like this:

data class QTag(val rawQTag: String?) {
    val affiliateId: String?
    val creativeId: String?
    val trackerId: String?

    init {
        // build intermediate list of strings to perform search later
        val strs = rawQTag.orEmpty().lowercase()
            .split("_")
            .filter { it.length > 1 }
        affiliateId = strs.findIdByPrefix("a")
        creativeId = strs.findIdByPrefix("c")
        trackerId = strs.findIdByPrefix("t")
    }

    private fun List<String>.findIdByPrefix(prefix: String): String? = firstNotNullOfOrNull {
        if (it.substring(0, 1) == prefix) it.substring(1) else null
    }
}

[Tim Oltjenbruns]

I’m late, but you could also use associateBy to output a map of items by prefix. Then afterwards assign the item by prefix into the appropriate variables.

[Tim Oltjenbruns]

data class QTag(val rawQTag: String?) {
    val affiliateId: String?
    val creativeId: String?
    val trackerId: String?

    init {
        // build intermediate list of strings to perform search later
        val substringsByPrefix = rawQTag.orEmpty().lowercase()
            .split("_")
            .filter { it.length > 1 }
            .reversed()
            .associateBy { it[0] }
        affiliateId = substringsByPrefix.getWithoutPrefix('a')
        creativeId = substringsByPrefix.getWithoutPrefix('c')
        trackerId = substringsByPrefix.getWithoutPrefix('t')
    }

    private fun Map<Char, String>.getWithoutPrefix(c: Char) = this[c]?.substring(1)

}

The firstOfNotNull works too. It seems a little less clear to me, but with the potential benefit of being more performant if you expect most cases to have way more substrings than you are parsing.

If you expect the typical case to include exactly these substrings and no more, then the potential benefit of the firstNotNullOfOrNull approach does not apply.

[Fleshgrinder]

If performance really matters then all of the proposed solutions are going to be slower than a standard loop. Doing something simple as split("_") already results in an additional list allocation, all additional operations like substring and associate also result in additional allocations. If raw speed matters than nothing like this should be done, and instead the string should be parsed.