Curious on people’s opinions for extension functio...
# announcements
j
Curious on people’s opinions for extension function best practices. In my mind, they’re good for extending classes you don’t have access to, either to add methods or make them null-safe, or want to extend in a very limited scope with private extension functions. One thing I noticed is sometimes I see people use variables from the scope the extension function is defined in, which feels weird to me. I tried to draft a “rule” of sorts for us to use at work, and was wondering how others feel extension functions should be used. (This is especially relevant as we use Guice everywhere in our server app)
Extension functions should not use variables from a scope other than the scope available to them from the class they’re extending. Instead, variables should be passed in as parameters. Put another way, extension functions should be “stateless”, and not rely on where they’re defined to function. Extension functions should be able to be moved to a new file and continue to work.
c
That would break some of my own goodies 🙂 e.g. in a Spring Boot app I have an entity
MyEntity: Entity
and repository:
MyRepository
I don't want to add an explicit dependency on the repository so I define a "autowired" extension function
Entity.save()
, which uses repository from its defintion scope, then I'm able to write:
Copy code
MyEntity(...).save()
j
Interesting… that almost seems to get in to implicits territory, a la Scala, right?
c
I couldn't say, I haven't worked with Scala beyond some basics several years ago.
j
So the alternative in your case is to inject
MyRepository
wherever you need to call
Entity.save()
c
yup and call
myRepository.save(MyEntity(...))
s
You'd like this article: “An introduction context-oriented programming in Kotlin” by Alexander Nozik https://link.medium.com/o1oQOJMqpU
c
This also gives me a nice separation of concerns, when refactoring, if I change data storage implementation, I don't need to change any client code, only the extension function implementation.
j
@streetsofboston this bit seems particularly relevant
This means that the logic inside context is completely separated from the context implementation and could be written in different part of the program or even different module.
💯 1
@Czar you could accomplish the same thing by injecting a wrapper around your store implementation, and then refactor that as necessary as well right?
c
I could, and I argue that my
Entity.save()
function is that wrapper 🙂
d
I do feel a bit icky when my extension functions pull in context from their lexical scope, and try to define them at the top level. That said, I quite often suppress the nausea if it makes things more expressive.
j
Right. I think what I’m trying to come up with a decent heuristic for, is when do you use so many variables from the surrounding context that the function should be on the class you’re using it in instead of an extension function.
i.e. instead of
Copy code
class Bar @Inject constructor(
  private val a: A,
  private val b: B,
  private val c: C
) {
  fun handle(foo: Foo) {
    return foo.handle()
  }

  fun Foo.handle() {
    return this.sumWithA(a) + this.sumWithB(b) + this.sumWithC(c)
  }
}
That seems like it should just be a private function on class Bar, instead of an extension function on class Foo
@dmcg do you think passing context from the outer scope makes it less icky, or does it reduce expressiveness too much?
d
In that case I don't think that the extension does anything for you. But for handling nulls and chaining calls they can really help - http://oneeyedmen.com/extension-functions-can-be-utility-functions.html
j
Yep, agreed. It’s probably worth pointing out none of those examples use the context they’re defined in, instead using the context provided to them by the class they’re extending.
d
Yes. It's when you want the chaining and the closure over local state to avoid passing lots of
this.a, this.b
params that you get the combination. In these cases I make them private and suck it up.
j
So if you have to pass the variables, what makes it an extension function rather than a function in the same scope that
a
and
b
are defined?
d
In your class above, say I had
Copy code
fun handle(foo: Foo) = op2(op1(foo, a, b), a, b)
then I might well define
Copy code
fun Foo.op1() = doAThingWith(foo, a, b)
fun Foo.op2() = anotherThingWith(foo, a, b)
so that I could write
Copy code
fun handle(foo: Foo)  = foo.op1().op2()
j
Right, instead of
Copy code
fun handle(foo: Foo)  = foo.op1(a, b).op2(a, b)
Though arguably that one’s more explicit at the cost of extra characters for the parameters. Of course, if you only had
op1()
, so no chaining, then an extension function would make less sense under your guidelines yeah?
d
Yes. Two parameters might be fine in context - I'm quite sensitive to the cognitive load. I'd do this if I wanted the reader to focus on the operations not the mechanics
In the case of only one op I think I'd only do this for
.toSomethingElse()
rather than
convertToBar(foo)
-
foo.toBar()
j
Totally agree. Increasing cognitive overhead to make things nicer to write at the cost of making it harder to grok later is a big no in my books.
Right. And presumably it’d be fairly rare the conversion needs another context to work (at least with the conversions I do)
d
Somethings things like rootURL and namespace, but I'd normally pass those in
j
Right, especially for conversion functions it really helps reduce the overhead to read them. Otherwise conversion is dependent on context, and you have to figure out what’s implicitly being used.
d
I think I was saying the opposite about the load though - the extension functions with closure may make it so that you can cope with reading the code and understanding what it does in one pass, where if you had added in the parameters the reader would have baulked at the apparent complexity
If the reader later has to edit the code then they may curse me, but the ability to grok what a thing does in one pass may be worth the complexity
j
Probably depends on what’s implicitly being used, and if the function makes it clear. I guess I’m not convinced something like
Copy code
foo.op1().op2()
Is easier to grok the basic understanding of versus
Copy code
foo.op1(a, b).op2(a, b)
Except now I have to figure out what op1 and op2 are depending on implicitly, versus being able to see at the call site.
d
Yep. It depends. I probably have 4 pieces of Kotlin using the technique in 4 years
j
4 using which technique?
d
Extension functions capturing values from their enclosing scope
j
Ah, so it’s very much the exception for you.
d
Like I say - it makes me feel icky!
j
Haha, I agree! Hence trying to come up with some heuristic people on the team can follow, especially since we want to add a lot of people this year and they might not be super familiar with Kotlin. Trying to save them from the pointy edges to make sure we can write uniform code in a way.
As an aside, we’re also trying to do this with functions like
apply
,
run
,
let
, etc.
d
Then I agree with your original rule - but wouldn't replace
should
with
must
j
You’re saying keep it as “should” to allow some leeway?
Instead of using “must” which might be too strict.
d
Yes. You'd want your developers to think twice, but to be able to do the right thing if they could persuade others that it was
j
Yep, that makes sense. We’re going to try to do the same thing for cases like when should you use
if
instead of
apply
or
run
. Those can be a bit overwhelming for newer developers as well.