In my past I have it in my head that is is good pr...
# codingconventions
b
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?
nope 5
m
You can surround your code block with triple backtics (```) to get a single code block, instead of one codeblock per line.
b
@marstran Sorry, don't quite follow. I hit return too early so you may have been reading the incomplete post.
m
No, it's still 1 codeblock per line now. You can get this instead with triple backticks:
Copy code
fun dropLoot(name: String): Boolean {
   for (item in inventory) {
       if (item.name == name) {
           inventory.remove(item)
           return true
       }
   }
   return false
}
a
I’d definitely do two returns instead of a break
b
@marstran sorry, that looks exactly the same as my edited post.
m
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.
a
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
b
@August Lilleaas is part of a tutorial, this bit is concentrated on loops. Only on second week but kind of get what you mean.
a
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:
Copy code
fun dropLoot(name: String): Boolean {
   return inventory.removeIf { it.name == name }
}
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 🙂
s
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.
a
oh, my removeIf implementation is not equivalent btw. removeIf returns all matches, not just the first one
b
@Sam Stone good point. The answer is always it depends 😀
@August Lilleaas you know the tutorial 😀. This is the challenge when you just remove one of the items based on its name.
s
Also:
<https://kotlinlang.slack.com/archives/C4GKV43N2/p1661343206896469?thread_ts=1661342879.769889&cid=C4GKV43N2
|I hit return too early>
glorious irony
b
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
t
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
.
(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
.)