Add TypedOptions #1417
Add TypedOptions #1417
Conversation
| * | ||
| * Also consider [Options] to select an integer index. | ||
| */ | ||
| class TypedOptions<T : Any>( |
There was a problem hiding this comment.
Public constructor so we can accept an inline function in TypedOptions.of()
|
|
||
| companion object { | ||
| @JvmStatic | ||
| inline fun <T : Any> of( |
There was a problem hiding this comment.
This API isn’t totally Kotlin-idiomatic, but it’s consistent with Options
There was a problem hiding this comment.
We could have it be an invoke operator with a JvmName("of") annotation. I saw that in some other library I think
There was a problem hiding this comment.
Yep. I think keeping it consistent with Options is good.
| * limitations under the License. | ||
| */ | ||
|
|
||
| @file:JvmName("-BufferedSource") // A leading '-' hides this class from Java. |
There was a problem hiding this comment.
This is our first function that uses the same implementation for both Buffer and BufferedSource. It could be an extension function, but I think polymorphic is a little nicer, especially since the other select() is polymorphic
| * ``` | ||
| * TypedOptions<Direction> options = TypedOptions.of( | ||
| * Arrays.asList(Direction.values()), | ||
| * (direction) -> ByteString.encodeUtf8(direction.name().toLowerCase(Locale.ROOT)) |
There was a problem hiding this comment.
Java language example!
| val list = values.toList() | ||
| val options = Options.of(*list.map { encode(it) }.toTypedArray<ByteString>()) | ||
| return TypedOptions(list, options) |
There was a problem hiding this comment.
The amount of copies that occur in these three lines is staggering. I'm doing my best to suppress the desire to reduce them 🥵
There was a problem hiding this comment.
I didn’t see anything where reducing allocations was worth the extra code that it’d cost. I think it’s fine for Options to do defensive copies the expensive way. It’d be nice if we had a proper immutable list class instead of our janky unmodifiable list interface.
Co-authored-by: Jake Wharton <jw@squareup.com>
No description provided.