Looking for feedback on naming and implementation....

# codereview

[Mark]

Looking for feedback on naming and implementation. If all items transform to the same value, then that value is returned, otherwise null

fun <T, R> Iterable<T>.singleOrNullBy(transform: (T) -> R): R? {
    if (this is Collection && this.isEmpty()) return null
    return asSequence()
        .map(transform)
        .zipWithNext()
        .map { (a, b) -> if (a == b) a else null } // important we don't use mapNotNull here
        .distinct()
        .singleOrNull()
}

[ephemient]

this can be inlined and returns early when possible:

inline fun <T, R> Iterable<T>.singleOrNullBy(transform: (T) -> R): R? {
    return fold(null) { acc: R?, item ->
        val value = transform(item)
        if (acc == null || acc == value) value else return null
    }
}

[ephemient]

although that doesn't quite handle null the same way; if you need that, a simple sentinel can do:

inline fun <T, R> Iterable<T>.singleOrNullBy(transform: (T) -> R): R? {
    val nothing = Any()
    return fold(nothing) { acc: Any?, item ->
        val value = transform(item)
        if (acc === nothing || acc == value) value else return null
    }.takeIf { it !== nothing } as R?
}

[ephemient]

although as it returns the transformed value, not the original, I think it would be more consistently named singleOfOrNull

[Mark]

Thanks @ephemient I hadn’t thought of jumping out with a return like that. How about using an Iterator?

inline fun <T, R> Iterable<T>.singleOfOrNull(transform: (T) -> R): R? {
    // let's see if we can avoid creating the iterator
    if (this is Collection) {
        if (this.isEmpty()) return null
        if (this is List && size == 1) {
            return transform(this[0])
        }
    }
    return this.iterator().singleOfOrNull(transform)
}

inline fun <T, R> Iterator<T>.singleOfOrNull(transform: (T) -> R): R? {
    if (!this.hasNext()) {
        return null
    }
    val firstItemTransformed = transform(this.next())
    if (!this.hasNext()) {
        return firstItemTransformed
    }
    this.forEach { item ->
        if (transform(item) != firstItemTransformed) {
            return null
        }
    }
    return firstItemTransformed
}

[Mark]

But then again, might as well just do this (although perhaps not as performant):

inline fun <T, R> Iterable<T>.singleOfOrNull(noinline transform: (T) -> R): R? {
    // let's see if we can avoid creating the sequence
    if (this is Collection) {
        if (this.isEmpty()) return null
        if (this is List && this.size == 1) {
            return transform(this[0])
        }
    }
    return this.asSequence().map(transform).distinct().singleOrNull()
}

Regarding naming, maybe singleDistinctOfOrNull because single by itself doesn’t say anything about distinct values.

[ephemient]

if you're going to use a Sequence, just don't make the extension function inline to begin with, and see my other comment about being safe wrt. TOCTTOU and mutable collections. but sure, that's a reasonable name and straightforward implementation

[ephemient]

note that you can also write

inline fun <T, R> Iterable<T>.singleOfOrNull(transform: (T) -> R): R? {
    val iterator = iterator()
    if (!iterator.hasNext()) return null
    val single = transform(iterator.next())
    for (item in iterator) if (transform(item) != single) return null
    return single
}

as Kotlin defines Iterator.iterator() to return itself

[ephemient]

(by TOCTTOU issue, I mean that

val list = CopyOnWriteArrayList(listOf(1))
thread { list.add(2); list.add(3); list.remove(1) }
list.singleOfOrNull { it }

could conceivably return 2 in your implementation, even though there is never a point in time where list == listOf(2))

[Mark]

Isn’t this the same as my Iterator -based implementation but without the quick checks?
https://kotlinlang.slack.com/archives/C1H43FDRB/p1654337100738409?thread_ts=1654317012.586139&cid=C1H43FDRB

[ephemient]

1. changed .forEach to for
2. your second quick check is vulnerable to TOCTTOU

[Mark]

inline fun <T, R> Iterable<T>.singleDistinctOfOrNull(transform: (T) -> R): R? {
    // let's see if we can avoid creating the iterator
    if (this is Collection && this.isEmpty()) {
        return null
    }
    val iterator = this.iterator()
    if (!iterator.hasNext()) {
        return null
    }
    val firstItemTransformed = transform(iterator.next())
    if (!iterator.hasNext()) {
        return firstItemTransformed
    }
    for (item in iterator) if (transform(item) != firstItemTransformed) return null
    return firstItemTransformed
}

[ephemient]

sure, and you don't need the is List check either, but you aren't saving anything: .singleOrNull() uses iterator() itself

[Mark]

Ah yes, very good points and well spotted regarding singleOrNull() using Iterator - I’ve just updated it.