New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kotlin extension properties for the Okio.<...>() helper methods #352
Comments
Please don't do this! Each call to buffered() creates a new Buffer, and that's both inefficient and incorrect. |
What about extension functions instead of fun Source.buffer(): BufferedSource = Okio.buffer(this)
fun Sink.buffer(): BufferedSink = Okio.buffer(this) That way you the code can be readable while being clear about Don't get me wrong, I know that one should not write something like this: val sink = ...
sink.buffered.write(...)
sink.buffered.write(...)
sink.buffered.write(...) ... but rather this val sink = ...
val bufferedSink = sink.buffered
bufferedSink.write(...)
bufferedSink.write(...)
bufferedSink.write(...) |
Also something like this in Kotlin: @file:JvmName("Okio")
package okio
import ...
fun Sink.buffer(): BufferedSink = RealBufferedSink(this)
val File.sink: Sink
@JvmName("sink") get() {
return FileOutputStream(this).sink
}
val OutputStream.sink: Sink
@Throws(FileNotFoundException::class)
@JvmName("sink")
get() {
...
} ... would actually compile to this in Java: package okio;
import ...;
public final class Okio {
@NotNull
public static final BufferedSink buffer(@NotNull Sink $receiver) {
return new RealBufferedSink($receiver);
}
@NotNull
public static final Sink sink(@NotNull File $receiver) {
return sink(new FileOutputStream($receiver));
}
@NotNull
public static final Sink sink(@NotNull OutputStream $receiver) throws FileNotFoundException {
Intrinsics.checkParameterIsNotNull($receiver, "$receiver");
...
}
} So when changing Okio's static utility methods to extension functions or extension properties, the Java code would still be the same, thus not breaking compatibility to older Okio versions. |
It wouldn't be hard to transform the whole |
We don't want the dependency on the Kotlin stdlib.
…On Sat, Feb 24, 2018, 11:10 AM Heinrich Reimer ***@***.***> wrote:
It wouldn't be hard to transform the whole Okio class into a Kotlin file
with extension functions/properties that would compile to the same JVM
signature as the current implementation but at the same time being more
readable when writing Kotlin code.
If you'd like, I could submit a pull request.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#352 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEf9nssKOrYmisF4ro3M12MPzwhAEks5tYDR8gaJpZM4SR6NP>
.
|
@JakeWharton That's reasonable. What about a section in the Readme about Kotlin compatibility? |
Well you should import those functions to clean it up a bit, but I'm still
not sure what we'd put in the README. There's nothing wrong with that style
of function calls in Kotlin. And if you want to call them after the subject
you can write those extensions in your project or even do
file.let(::sink).let(::buffer). We could ship a Kotlin artifact that adds
these functions, but I would want a bit more value from it than just these.
…On Sat, Feb 24, 2018, 11:19 AM Heinrich Reimer ***@***.***> wrote:
@JakeWharton <https://github.com/jakewharton> That's reasonable.
What about a section in the Readme about Kotlin compatibility?
It just seems to me that writing something like Okio.buffer(Okio.sink(to))
is not a good code style in Kotlin when you could call to.sink().buffer()
or something similar instead...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#352 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEUJX-lSnZzSyO6C1McBZFK88PwMTks5tYDaWgaJpZM4SR6NP>
.
|
@JakeWharton Agreed! I'll continue collecting Kotlin-specific "syntactic sugar" here until we have a reasonable value to create a separate artifact. So for now the list would be:
|
If you don't use any stdlib functions, Kotlin compiles to pure byte-code with no runtime dependency. Alternatively, if you wish to use Kotlin classes you can invoke kotlinc in a way ( Not that I care one way or another, just sayin' ... |
Your first statement is in accurate. The declaration of a non-null
parameter or calling a Java method that you coerce the platform type into a
non-null type will emit a call to the Intrinsics class. Any proposed
extension would be on a non-null receiver thus causing this dependency
immediately even though we never explicitly reference the stdlib. Even if
it's marked inline that doesn't help Java callers from executing the
bytecode with the Intrinsics reference.
Also 200kb is nearly triple the size of the entire library.
…On Sat, Feb 24, 2018, 10:28 PM Brett Wooldridge ***@***.***> wrote:
@JakeWharton <https://github.com/jakewharton>
We don't want the dependency on the Kotlin stdlib.
If you don't use any stdlib functions, Kotlin compiles to pure byte-code
with no runtime dependency.
Alternatively, if you wish to use Kotlin classes you can invoke kotlinc in
a way (-includeRuntime) that builds a jar which includes your classes and
kotlin runtime classes, so there is no external dependency. The cost is
~200Kb.
Not that I care one way or another, just sayin' ...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#352 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEERRJc9h34BNCrZMRPZQm9uTQmxIWks5tYNNvgaJpZM4SR6NP>
.
|
@JakeWharton ... except when you compile with |
That removes most, but not all. We've explored this before!
…On Sat, Feb 24, 2018, 10:37 PM Brett Wooldridge ***@***.***> wrote:
@JakeWharton <https://github.com/jakewharton> ... except when you compile
with -Xno-param-assertions and -Xno-call-assertions.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#352 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEYqCuCw4_CChyTQATWXKddl2vrLmks5tYNWEgaJpZM4SR6NP>
.
|
@JakeWharton Ok. Do you recall what dependencies remained? |
I'll have to check tomorrow. I spent some time investigating it for
android-ktx project. I'd be happy to figure out a way to eliminate more of
these interop checks.
It's worth noting, though, that we actually would want the parameter checks
here in Okio. A better solution is probably using ProGuard to strip and
rename the unused stdlib methods which would likely only leave is with one
or two methods from Intrinsics.
…On Sun, Feb 25, 2018, 3:48 AM Brett Wooldridge ***@***.***> wrote:
@JakeWharton <https://github.com/jakewharton> Ok. Do you recall what
dependencies remained?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#352 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEYhQYCJD3AvNemmNgumi4Iz7Z-xDks5tYR53gaJpZM4SR6NP>
.
|
Obsoleted by our Okio 2 effort. |
Would be nice for readability if we could use Kotlin extension properties for not having to nest the
Okio.source(...)
,Okio.sink(...)
,Okio.buffer(...)
methods.That way one could intuitively navigate to a
File
'sBufferedSink
usingfile.sink.buffered
.@swankjesse's Okio file copy example on Twitter would then be reduced to this:
Extension properties
The text was updated successfully, but these errors were encountered: