In my past I have it in my head that is is good practice just to have one return in a function so ra...

# codingconventions

[Ben Edwards]

In my past I have it in my head that is is good practice just to have one return in a function so rather than do
*fun* dropLoot(name: String): Boolean {
*for* (item *in inventory*) {
*if* (item.*name* == name) {
*inventory*.remove(item)
return true
}
}
return false
}
I would do
*fun* dropLoot(name: String): Boolean {
*var* ret = *false*
*for* (item *in inventory*) {
*if* (item.*name* == name) {
*inventory*.remove(item)
ret = *true*
break
}
*return* ret
}
What do people think?

[marstran]

You can surround your code block with triple backtics (```) to get a single code block, instead of one codeblock per line.

[Ben Edwards]

@marstran Sorry, don't quite follow. I hit return too early so you may have been reading the incomplete post.

[marstran]

No, it's still 1 codeblock per line now. You can get this instead with triple backticks:

fun dropLoot(name: String): Boolean {
   for (item in inventory) {
       if (item.name == name) {
           inventory.remove(item)
           return true
       }
   }
   return false
}

[August Lilleaas]

I’d definitely do two returns instead of a break

[Ben Edwards]

@marstran sorry, that looks exactly the same as my edited post.

[marstran]

Anyway, for your question. I'm a fan of guard clauses and early returns, so I would have written it like the first example. I just think it's easier to read when I get the edge cases out of the way early.

[August Lilleaas]

not really related to your question, but I’d also pass in the game state (this looks like a game at least) to the function, instead of referring to inventory

[Ben Edwards]

@August Lilleaas is part of a tutorial, this bit is concentrated on loops. Only on second week but kind of get what you mean.

[August Lilleaas]

also, imo an improved version of dropLoot would be to just use the functions in the stdlib. You would get the same effect with this:

fun dropLoot(name: String): Boolean {
   return inventory.removeIf { it.name == name }
}

[August Lilleaas]

right, sounds like a good idea to follow the tutorial and not try to be too smart, like I’m trying to be right now 🙂

[Sam]

I think there are good arguments on both sides in the early returns debate. I don’t tend to apply a blanket rule, but try to use whichever approach feels more readable in the context. I do try to avoid using break/continue in loops, though! I find that more disorienting than an early return. So in the specific case that you shared, I prefer the early return.

[August Lilleaas]

oh, my removeIf implementation is not equivalent btw. removeIf returns all matches, not just the first one

[Ben Edwards]

@Sam Stone good point. The answer is always it depends 😀

[Ben Edwards]

@August Lilleaas you know the tutorial 😀. This is the challenge when you just remove one of the items based on its name.

[Sam]

Also:

<https://kotlinlang.slack.com/archives/C4GKV43N2/p1661343206896469?thread_ts=1661342879.769889&cid=C4GKV43N2

|I hit return too early>

glorious irony

[Ben Edwards]

As a side note, I am trying to use Kotlin rather the more direct Java versions. So *inventory*._removeAll_ *{ it*.*name* == name *} with* mutableListOf rather than ListArray.removeIf

[Tom Yuval]

Another point not mentioned above: lacking a good reason to do otherwise, prefer immutability to mutability. In this case, I’d choose the option that does not depend on a var.

[Tom Yuval]

(Also, I question the value in striving for a single return in the first place. This doesn’t seem to me like something that increases readability or maintainability. I guess it’s not impossible for there to be some good arguments for it, but whatever negative aspect there might be to multiple `return`s I guess it’s at least as bad when returning a var.)