Which approach do you think is more idiomatic/more...

# getting-started

[CLOVIS]

Which approach do you think is more idiomatic/more readable?

fun makeUnique(name: String): String {
    var candidate = name

    var counter = 1
    while (alreadyExists(candidate)) {
        candidate += "-$counter"
        counter++
    }

    return candidate
}

• Looks like what you'd write in Java
• More similar to what you'd see in algorithm books

fun makeUnique(name: String): String = sequence {
    yield(name)

    for (i in 1..Int.MAX_VALUE) {
        yield("$name-$it")
    }

    error("Could not find a unique name")
}.first { !alreadyExists(it) }

• The condition at the end is the result you're expecting (unlike in the first version which tests the opposite of what it wants)
• It's easier to test: the sequence generator could be extracted to another method, allowing to write a unit test of the exact candidates attempted
• It encourages to developer to notice the potential overflow / it has a nice place to put the guardrail
• No mutability (easier to debug? though sequences are not great to debug at the moment)

[ephemient]

they don't do the same thing. the first can produce $name-1-2

[CLOVIS]

Ouch, sorry, my bad, it's an error when retyping it. I'll edit it, my question is purely about the style

[CLOVIS]

*edited

[ephemient]

well, it depends IMO. one downside to the second is that you can't use other suspend functions inside sequence operators

[CLOVIS]

If I wanted to use suspend I could just replace sequence {} by flow {} and it would behave the same, right?

[ephemient]

yes, although I think the stacktrace with seq might be more likely to be readable?

[ephemient]

anyhow personally I'd tend to do the imperative thing if it's simple and the functional thing if it's not

[CLOVIS]

In this case the real example is fairly close from this version, it's attempting to create a new file in a directory, changing the name to ensure it doesn't overwrite files that already exist

[ephemient]

(that sounds potentially racy)

[CLOVIS]

Ahah yeah, thought the same. Will be fine in this particular case, but it's definitely not a good implementation in a general case.

[CLOVIS]

But anyway, the question was mostly about imperative vs sequence-based

[Adam S]

from the examples you gave, I’d prefer the first imperative style. But I like sequences so I’d probably refactor the second to break it up into two functions which I prefer overall

fun namesSequence(name: String): Sequence<String> =
  sequence {
    yield(name)
    repeat(Int.MAX_VALUE) { i ->
      yield("$name-$i")
    }
  }

fun uniqueName(name: String): String =
  namesSequence(name)
    .firstOrNull { !alreadyExists(it) }
    ?: error("Could not find a unique name")

[CLOVIS]

I would still put the error in the first one, since it's written as an infinite sequence

[Magdalena Tsolaki]

Second

[ephemient]

fun names(name: String) = sequenceOf(name) + generateSequence(1) { it + 1 }.map { "$name-$it" }

[ephemient]

but also filesystem performance can start breaking down after several tens of thousands of files in the same directory so you're gonna run intoother issues before you worry about running out of ints

[ephemient]

this is why git uses the first couple characters of the object hash as a sharding key in its loose object store, for exalt

[Wout Werkman]

Please also note that Kotlin's generators with closables are not always safe.

sequence {
    file.inputStream().use { /* yields here */ }
}

Now if you don't fully exhaust this sequence, the finally block will not be executed.
(Not too relatable maybe, but this should be considered when you want to use generators)

[CLOVIS]

Thanks for the info, I'll try to remember it.